omarryhan/aiogoogle

Chat API {name=spaces/*} and unquoting issue

Closed this issue ยท 2 comments

How to reproduce:

creds = ServiceAccountCreds(scopes=["https://www.googleapis.com/auth/chat.bot"])
...
bot = await _client.discover("chat", "v1")
spaces = await _client.as_service_account(bot.spaces.list())

{'spaces': [{'name': 'spaces/_kWXNwAAAAE', 'type': 'DM', 'singleUserBotDm': True, 'threaded': False, 'displayName': ''}], 'nextPageToken': ''}

await _client.as_service_account(bot.spaces.get("spaces/_kWXNwAAAAE"))

aiogoogle.excs.HTTPError:

Not Found

Request URL:
https://chat.googleapis.com/v1/spaces%2F_kWXNwAAAAE

The URL doesn't exist because '/' is getting encoded as '%2F' though 'spaces/*' is a valid url param (reference).

Potential fix?

I suspect the issue starts at resource.py#L599:

sorted_required_path_params[k] = quote_plus(str(v))

dict_values(['spaces%2F_kWXNwAAAAE'])

Perhaps urllib.quote or quote_plus(string, safe='/') would fix it? For reference:

sorted_required_path_params[k] = quote(str(v))

dict_values(['spaces/_kWXNwAAAAE'])

... bot.spaces.get("spaces/_kWXNwAAAAE") ...

{'name': 'spaces/_kWXNwAAAAE', 'type': 'DM', 'singleUserBotDm': True, 'threaded': False, 'displayName': ''}

I didn't test all the use cases though (e.g. #47)

System Info

Thanks for the great lib!

Nice catch. I should've opted for quote instead of quote_plus. As highlighted by the urllib docs:

By default, this function is intended for quoting the path section of a URL.

Also, according to the urllib docs, the only two differences between quote_plus and quote is that:

  1. quote_plus replaces spaces with a + instead of a %20.
  2. It does not treat forwards slashes as safe chars.

So, I suspect that changing to quote might cause future data inconsistency for some of the users of this lib. E.g. a user created a reaource using an encoded forward slash, but now can't fetch it because forward slashes are being escaped. That's assuming that Google APIs accept URL data in non idempotent requests, which i'd say is unlikely.

Anyway, I think it's best to change it to the correct quote function and bump the version up by one major version to give people a heads up about the change. What do you think?

Thanks for raising the issue!

Sounds good to me! ๐Ÿ‘
Thanks.