Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resty: Add workaround for new curl versions #157

Merged
merged 3 commits into from
Apr 17, 2024
Merged

resty: Add workaround for new curl versions #157

merged 3 commits into from
Apr 17, 2024

Conversation

pothos
Copy link
Member

@pothos pothos commented Apr 16, 2024

As Mathieu found out, his new curl version won't process the empty string being passed to curl as argument, resulting from the no-body case for GET. The empty string was treated as additional URL which was skipped over in the past with a warning but now it's a hard error.

How to use

Testing done

With

touch /tmp/flatcar_production_update.gz
NOUPLOAD=1 ./upload_package /tmp https://staging.updateservice.flatcar-linux.net notused 9.9.9

I checked that the get_package_id shows the error for wrong curl arguments when switching the body_arg=() assignment to =(--unknown). It now also prints the curl errors when curl hits an HTTP error code which is the case for the final POST in the above test.

resty Outdated Show resolved Hide resolved
@pothos pothos force-pushed the kai/resty-curl branch 2 times, most recently from fb4cc95 to ea57f47 Compare April 16, 2024 14:19
pothos added 2 commits April 16, 2024 23:22
The get_package_id function was called with "if" which disabled set -e
and thus continued after resty returned an error. The resty function
itself doesn't want to be called with set -e and thus we have to wrap
the get_package_id call to use set -e while we can explictly react on
resty errors which disables set -e. So one time we should not use
"if"/"||" and one time we should. While at it, remove the tempfile
creation which resulted in many entries that weren't cleaned up and
which were confusing because resty also creates some and one doesn't
know which is which when debugging.
As Mathieu found out, his new curl version won't process the empty
string being passed to curl as argument, resulting from the no-body case
for GET. The empty string was treated as additional URL which was
skipped over in the past with a warning but now it's a hard error.
@pothos pothos requested a review from a team April 17, 2024 06:13
@pothos
Copy link
Member Author

pothos commented Apr 17, 2024

One can also do the test with NOUPLOAD=1 ./upload_package /tmp http://localhost:8000 notused 9.9.9 when running printf "HTTP/1.1 200 OK\r\n\r\n" |nc -l -p 8000; printf "HTTP/1.1 200 OK\r\n\r\n" | nc -l -p 8000 as server

Copy link
Contributor

@tormath1 tormath1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hacky... but it works. Thanks for making the upload_package a bit more robust.

@pothos pothos merged commit 58ff732 into master Apr 17, 2024
@pothos pothos deleted the kai/resty-curl branch April 17, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants