astarte-platform / astarte

Core Astarte Repository
https://docs.astarte-platform.org/
Apache License 2.0
240 stars 46 forks source link

AppEngine queries on group devices do not work if the group name contains a forward slash '/' #904

Closed Pavinati closed 6 months ago

Pavinati commented 8 months ago

Steps to replicate:

Expected result

The device data

Actual result

{"errors":{"detail":"Device not found"}}

rbino commented 8 months ago

Why %252F and not %2F? How does this behave if you replace %2F in both cases above?

Pavinati commented 8 months ago

Double encoding is used to avoid mistaking the / in abc/xyz as part of the path. The fact that one query works on the other does not makes me think this was addressed in the past but not for every group query

rbino commented 8 months ago

The issue is that InterfaseValuesByGroupController does not decode the group name like DeviceStatusByGroupController does.

Easy fix: add that decoding. Better fix: scope all /groups APIs and make a plug that fixes group_name in a single place so we don't miss out on (potentially new) APIs there.

Marking this as Good first issue since it's quite trivial (also the test section of groups is not terrible so it's reasonable to also add a test for the fix to avoid regressions).