-
Notifications
You must be signed in to change notification settings - Fork 54
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
ostree: fix proxy parsing (HMS-5018) #1050
Conversation
0306c80
to
45fed14
Compare
I did split the OTK hunks into a separate PR: #1051 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow, nice find :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice find! Thanks! 🎉
Would it make sense to add a regression test here? Also in the "test-as-documentation" sense this might be useful(?) |
It would make sense to add test for sure... I'd prefer a follow-up tho, I wouldn't like to block this, unless someone disagrees. |
I can do a followup, I actually tried by running a tiny HTTP proxy via library but it was not great test to be honest. |
Cool, we don't need to block on this, I had a quick look and opened #1052 - it's not ideal as it's not a full integration test but OTOH I think the extraction of the helper is beneficial and it does check that the proxy is correctly set (but it does know a bit too much about the implementation details for my taste). |
Finally found the ostree resolving problem - it was in proxy parsing. The configuration value is not in URL form, it is in
hostname:port
form. Andurl.Parse
silently parses such URI setting a nonsense scheme tohostname
. TIL thatURL
in Go actually is technically URI: https://pkg.go.dev/net/url