mosquito / aiormq

Pure python AMQP 0.9.1 asynchronous client library
Other
268 stars 58 forks source link

hotfix: unquoting vhost in path #202

Closed ofekashery closed 4 weeks ago

ofekashery commented 4 weeks ago

On August 30, 2024, yarl released version 1.9.5. On this version, they introduced breaking changes for parsing the %2F char in path, which is frequently used by vhosts in the AMQP connection strings.

url = yarl.URL("amqp://localhost/%2f")
print(url.path)
# Yarl v1.9.4 -> `//` -> self.vhost = '/' 
# Yarl v1.9.5 -> `/%2F` -> self.vhost = '%2F'

This bug broke our critical production system, which relied on %2F as the vhost and led into protocol issues. I just wrote an quick hotfix for that issue that allows connect successfully to the RMQ with both yarl versions.

Fixes #203.

ofekashery commented 4 weeks ago

Thank you, @mosquito ❤️

Dreamsorcerer commented 1 week ago

FYI, we've decided to revert this in yarl as it's caused a few more issues. If you need to know the path separators for safety, we're adding a path_safe attribute instead which skips decoding %2F and %25.

decaz commented 1 week ago

@ofekashery @mosquito due to information from @Dreamsorcerer - should this PR be reverted or these changes can be left?

mosquito commented 1 week ago

Probably not.

Dreamsorcerer commented 1 week ago

Not reverting would create 2 minor issues. First is obviously just performance, it's a wasted call. Second is the possibility of double unquoting, e.g. %252F would now become /, when it should really be %2F.