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

Accept tuple lists in set_resp_headers #1295

Open
essen opened this issue Jun 4, 2018 · 10 comments
Open

Accept tuple lists in set_resp_headers #1295

essen opened this issue Jun 4, 2018 · 10 comments

Comments

@essen
Copy link
Member

essen commented Jun 4, 2018

To make it easier to deal with different implementations (like Gun).

#1287 (comment)

@geeksilva97
Copy link
Contributor

Am I right to assume you meant something like the following?

Req = cowboy_req:set_resp_headers([
    {<<"content-type">> , <<"text/html">>},
    {<<"content-encoding">>, <<"gzip">>}
], Req0).

@essen
Copy link
Member Author

essen commented Feb 5, 2024

Yes.

@geeksilva97
Copy link
Contributor

Will we still do this merge? What should happen in this scenario?

Req = cowboy_req:set_resp_headers([
    {<<"set-cookie">> , <<"name=essen">>},
    {<<"set-cookie">>, <<"age=28">>}
], Req0).

I would say it should call the set_resp_cookie twice. Does it make sense?

@essen
Copy link
Member Author

essen commented Feb 6, 2024

In the case of set-cookie it should not call set_resp_cookie but rather append the cookie to resp_cookie (like set_resp_cookie does, but without having to parse then rebuild the cookie).

Because there can be duplicates of other headers, the headers should first be processed a little: set-cookie can be handled here, and then duplicate headers must be consolidated (Val1 + ", " + Val2). Then the result can be made into a map and merged (overriding existing headers if there's already been a duplicate set).

It probably should not be named set_resp_headers since the behavior differs, but set_resp_headers_list or something.

@geeksilva97
Copy link
Contributor

Got it

@voluntas
Copy link
Contributor

voluntas commented Apr 15, 2024

I want to include multiple Link headers in an HTTP response, but it seems that the latest version of Cowboy does not allow returning multiple headers.

I understand that this Issue and this Pull Request address this issue.
#1634

Is there anything blocking the implementation of this feature?

Additional note: It is defined that separating values with commas when outputting can achieve the same result as multiple disbursements, so it is possible to avoid the issue using this method.

Reference

The following mechanism requires adding duplicate headers.
https://www.ietf.org/archive/id/draft-ietf-wish-whip-01.html#section-4.4

Link: stun:stun.example.net;
Link: turn:turn.example.net?transport=udp; rel="ice-server"; username="user"; credential: "myPassword"; credential-type: "password";
Link: turn:turn.example.net?transport=tcp; rel="ice-server"; username="user"; credential: "myPassword"; credential-type: "password";
Link: turns:turn.example.net?transport=tcp; rel="ice-server"; username="user"; credential: "myPassword"; credential-type: "password";

@geeksilva97
Copy link
Contributor

Sup @voluntas . No blocker. I just couldn't start working on it. If you want to take a look, go ahead.

@essen
Copy link
Member Author

essen commented Apr 16, 2024

The goal of this ticket is not to add support for multiple of the same header. Hopefully that's not needed; if it is then we'd need to have a separate interface for this.

You SHOULD be able to have multiple links in one Link header:

   Link: </TheBook/chapter2>;
         rel="previous"; title*=UTF-8'de'letztes%20Kapitel,
         </TheBook/chapter4>;
         rel="next"; title*=UTF-8'de'n%c3%a4chstes%20Kapitel

https://datatracker.ietf.org/doc/html/rfc8288#section-3.5

But your Link headers are not including the < > so they might not follow the spec in other ways.

@essen
Copy link
Member Author

essen commented Apr 16, 2024

Note that the draft you linked is outdated. The most recent draft has correct Link headers: https://www.ietf.org/archive/id/draft-ietf-wish-whip-13.html#section-4.4

You should be able to use a single Link header to convey the same information.

@voluntas
Copy link
Contributor

Ah, it seems like I posted an old RFC draft, sorry about that.

I understand that one Link header should include multiple links.
Personally, I think it should be handled with a single header as well.

For now, I'll withdraw this request. Thank you always for your polite response!

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

3 participants