seratch / notion-sdk-jvm

A Notion SDK for Any JVM Language
https://developers.notion.com/
MIT License
136 stars 25 forks source link

Notion's API returns URL encoded PageProperty's ids #60

Open looorent opened 2 years ago

looorent commented 2 years ago

When fetching a page from Notion's API (https://api.notion.com/v1/pages/:id), some property ids can contain special characters (https://developers.notion.com/reference/page).

For example: t%7B%7Di, zl%7Da. Notion's documentation does not state if these values are URL encoded or not. However, this line re-encodes the property id: https://github.com/seratch/notion-sdk-jvm/blob/e46225782a369e4ee23c4e1a8ed6c7549d3aa524/core/src/main/kotlin/notion/api/v1/endpoint/PagesSupport.kt#L173

This is particularly a problem since Notion's version 2022-06-28, which forces the use of https://api.notion.com/v1/pages/:id/properties/:propertyId.

I am wondering whether the property id must be encoded when using retrievePagePropertyItem or not. Or, maybe PageProperty.id should be URL-decoded when deserializing. https://github.com/seratch/notion-sdk-jvm/blob/e46225782a369e4ee23c4e1a8ed6c7549d3aa524/core/src/main/kotlin/notion/api/v1/model/pages/PageProperty.kt#L12

My current workaround is to URL-decode the property id before using retrievePagePropertyItem.

seratch commented 2 years ago

Hi @looorent, thanks for writing in. This is interesting. The reason why I do the URL encoding for all the path parameters is that it is a commonly recommended practice for safety in web apps. I will look into this when I have time but your suggestions would be greatly appreciated too.

looorent commented 2 years ago

I have read https://developers.notion.com/reference/property-object, which does confirm the ID can have special characters, but it does not give more details. This kind of id looks common (half my property ids follow this format), which makes me think this will likely cause more issues in the future.

Maybe the most pragmatic (and unpopular) way to solve this is to not URL-encode this path parameter.

I have sent this question on Notion's Chat. They probably have a stronger opinion than me:

My question is about the format of the Property Ids exposed by the API. It seems they often contain some special characters (which is confirmed by https://developers.notion.com/reference/property-object). When retrieving a Page, here is an example of property id : t%7B%7Di.

However, these property ids must be passed as path parameters to some endpoints (for example, retrieving a page property). This is especially true since the new release 2022-06-28. Therefore, URL-encoding this path parameters is not compatible with their original format.

Do you advice to NOT url-encode this parameter? For example, here is a conversation about this subject: https://github.com/seratch/notion-sdk-jvm/issues/60

looorent commented 2 years ago

I have received this answer from Notion's technical support regarding the Property IDs returned with the Page endpoint:

Please consider them as already encoded.

So I guess they should be URL-decoded when deserializing the JSON.

seratch commented 2 years ago

Hi @looorent, I am sorry for my belated response here, and thanks a lot for sharing the information. Yes, I agree that this library can decode the value for users but am not sure what the best way to do it in a robust way is yet. I will keep this open but move it to future release milestones.