red-hat-storage / errata-tool

Modern Python API to Red Hat's Errata Tool
MIT License
31 stars 35 forks source link

Release class cannot query releases with "+" in names #207

Closed ktdreyer closed 3 years ago

ktdreyer commented 3 years ago

When we query a release with + in the name, we get NoReleaseFoundError:

rel = Release(name='RHEL-8.4.0.Z.MAIN+EUS')

Quoting the plus sign as %2B works.

rel = Release(name='RHEL-8.4.0.Z.MAIN%2BEUS')

We should make sure both of these continue to work so that we're fully backwards-compatible with existing code that quotes the "+" character. requests.utils.requote_uri() might be an option to preserve compatibility.

This might be a server-side bug - needs further investigation. (See CLOUDWF-1, possibly related?)

simonbaird commented 3 years ago

I think it's expected that literal '+' chars need to be escaped in a url param, otherwise they get interpreted as spaces. See https://stackoverflow.com/questions/2322764/what-characters-must-be-escaped-in-an-http-query-string for example.

ktdreyer commented 3 years ago

Oh, thanks Simon.

ktdreyer commented 3 years ago

These two requests show the problem:

requests.get('http://127.0.0.1:8000/?name=foo+bar')
requests.get('http://127.0.0.1:8000', params={'name': 'foo+bar'})

For the first GET request, requests sends that raw URL with a plus sign. For the second GET request, requests properly encodes the "+" value in the URL parameter.

127.0.0.1 - - [04/Jun/2021 13:25:59] "GET /?name=foo+bar HTTP/1.1" 200 -
127.0.0.1 - - [04/Jun/2021 13:25:59] "GET /?name=foo%2Bbar HTTP/1.1" 200 -

I'm still looking into how we can make this backwards compatible with users that might already be encoding the value there.

ktdreyer commented 3 years ago

This same problem is present in errata-tool-ansible. It fails to query resources if there is a + character in the name. RHELWF-3629 is the internal ticket for that.

EDIT: errata-tool-ansible fix was https://github.com/ktdreyer/errata-tool-ansible/pull/203

ktdreyer commented 3 years ago

For the record, these both work properly:

curl --negotiate -u : https://errata.stage.engineering.redhat.com/api/v1/releases/RHEL-8.4.0.Z.MAIN+EUS
curl --negotiate -u : https://errata.stage.engineering.redhat.com/api/v1/releases/RHEL-8.4.0.Z.MAIN%2BEUS
ktdreyer commented 3 years ago

Thanks @mzidek-gh for originally reporting this issue.