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

WIP: add headers support #178

Closed

Conversation

Carlotronics
Copy link
Contributor

Fixes #4 and #121.

WIP: Still needs to find a way to compare given value to each header values (currently only the first one is taken)

…tching

Uses preexisting any() function. 2 main drawbacks:
   - seems to break when using numeric operators ("<" for example);
  - makes impossible to use "," caracter in wanted header value
@TwiN
Copy link
Owner

TwiN commented Sep 25, 2021

Personally, I would prefer if you just retrieved each header using response.Header.Get(), put it in a map, and call it a day.
Out of all the time I've used Go, this has worked for all of my use cases, and surely it'll be enough to cover most use cases that everybody else may have.

@Carlotronics
Copy link
Contributor Author

The problem with response.Header.Get() is that you only get first header value. It will be enough in most cases, but not when working with response such as

HTTP/1.1 200 OK
[...]
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6)
User-Agent: AppleWebKit/537.36 (KHTML, like Gecko)

From RFC7230:

A recipient MAY combine multiple header fields with the same field
name into one "field-name: field-value" pair, without changing the
semantics of the message, by appending each subsequent field value to
the combined field value in order, separated by a comma.

Means that joining response.Header[headerKey] using a comma should not break any value (except for Set-Cookie which I should treat as a special case).

So the way I will implement it depends on the question: is it better to handle all cases and keep full response.Header[headerKey], or voluntarily exclude some of them for the sake of simplicity?

@TwiN
Copy link
Owner

TwiN commented Sep 28, 2021

Personally, I think we're fine with just checking the first header and keeping it at that.

If somebody opens up an issue in the future, that'll be a problem for then and there :P

@TwiN
Copy link
Owner

TwiN commented Mar 31, 2022

Since there has been no changes to this PR for over 6 months, I'm going to close it.
Thank you for the attempt, though, I appreciate it!

@TwiN TwiN closed this Mar 31, 2022
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.

Support header conditions
2 participants