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

Make yaws:accumulate_header/1 conforming to RFC #415

Open
leoliu opened this issue Mar 11, 2021 · 4 comments
Open

Make yaws:accumulate_header/1 conforming to RFC #415

leoliu opened this issue Mar 11, 2021 · 4 comments

Comments

@leoliu
Copy link
Contributor

leoliu commented Mar 11, 2021

Many headers allow you to sent as multiple headers with the same name for example, Vary, Cache-Control, Accept-Encoding, Set-Cookie etc etc. Not all of them are handled correctly.

I discovered this discrepancy while experimenting with the Vary header. In my case a request is passed through multiple functions. One function adds a Vary header for security reasons but the header is gone because a later function accidentally overrides it owing to yaws:accumulate_header/1's last-write-win semantics.

There is also the issue of not normalising the header names. For example, "Vary" and "vary" are different headers per Yaws.

@vinoski
Copy link
Collaborator

vinoski commented Mar 11, 2021

If you have multiple functions trying to contribute headers, you might consider using yaws_api:merge_header/2,3 instead:

    merge_header(Headers, {Header, Value})
          Merges value Value for header Header with any existing value for
          that  header in the #headers{} record Headers, and returns a new
          #headers{} record. Using the atom  undefined  for  Value  simply
          returns  Headers.  Otherwise,  Value is merged with any existing
          value already present in the Headers record for  header  Header,
          comma-separated  from  that  existing  value.  If  no such value
          exists in  the  Headers  record,  the  effect  is  the  same  as
          set_header/2.  Note  that  for the Set-Cookie header, values are
          not comma-separated but  are  instead  collected  into  a  tuple
          {multi,  ValueList}  where  ValueList  is the collection of Set-
          Cookie values. This implies that any formatting  fun  passed  to
          reformat_header/2 must be prepared to handle such tuples.

   merge_header(Headers, Header, Value)
          Same  as  merge_header/2  above, except Header and Value are not
          passed in a tuple.

Regarding "Vary" vs "vary", Yaws performs normalization of header names. Additionally, for some common headers like "Vary", you can use the atom vary so that normalization isn't an issue. If you can provide example code that produces "Vary" and "vary" together, or at least describe how to reproduce it, I'll take a look.

@leoliu
Copy link
Contributor Author

leoliu commented Mar 11, 2021

Sorry my original report may not be clear but I am referring to response headers.

@vinoski
Copy link
Collaborator

vinoski commented Mar 11, 2021

Sure, but as you probably know Yaws supports multiple approaches for server applications functionality, including .yaws pages, appmods, ehtml data, and more. It would be helpful if you could explain more about what your code is doing.

@leoliu
Copy link
Contributor Author

leoliu commented Mar 11, 2021

To simplify imagine an out/1 function that goes through a chain of many functions but eventually returns:

[...
 {header, {vary, "text1"}},
 ...
    ...
       {header, {vary, "text2"}},
 ...
]

Only the last vary header will be sent back because of the faulty last-write-win by yaws:accumulate_header/1.

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

No branches or pull requests

2 participants