canonical / surl

Ubuntu Store API thin wrapper
GNU General Public License v3.0
9 stars 11 forks source link

Fix bug when specifying headers multiple times #44

Closed shanepelletier closed 1 year ago

shanepelletier commented 1 year ago

Previously, when the user specified a header that was already specified by surl as a default header, the header would seemingly not get passed to curl. This particularly affected the passing of Content-Type:, but would also affect Accept:, User-Agent:, and Cache-Control:. This change merges the headers received from the user with the default headers (and with the headers previously specified, in the case the user specifies the same header multiple times on the command line), overwriting all values for the headers with the last value the user specifies.

This closes #43.

tartley commented 1 year ago

Hey! I know you've thought about this more than I have, but I feel obliged to ask the obvious question, if only to check I'm understanding correctly: Do we forsee any problems where surl used to provide a default header value, e.g. "Content-Type": "application/json", but now doesn't? Will any of the very small numbers of customers who use surl be confused by this behavior change?

Presumably, if it does cause a problem, then it's easy for users to workaround it by explicitly providing that header themselves to surl. But will it be obvious what users need to do?

Is there an alternative fix that doesn't come with this problem, ie: surl continues to provide the same default header values, but allows them to be overridden by the user passing in matching '-H' parameters.

Aaah, now that write this down, I'm seeing that this wouldn't be quite as trivial to implement as I first expected. I guess you'd have to:

Would this work? Is it desireable? Is it worthwhile? I'm happy to believe you if you say no, so long as we've discussed it.

shanepelletier commented 1 year ago

@tartley I originally wanted to avoid adding code if at all possible, but it turns out that even with my "fix", if a user specified the same header multiple times on the command-line, surl would not receive the expected response. Your suggestion fixes both issues, so I've changed things to hopefully work the correct way now.