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

sorted headers #1936

Open
fayland opened this issue Apr 12, 2022 · 16 comments
Open

sorted headers #1936

fayland opened this issue Apr 12, 2022 · 16 comments

Comments

@fayland
Copy link

fayland commented Apr 12, 2022

  • Mojolicious version: ALL
  • Perl version: ALL
  • Operating system: ALL

Steps to reproduce the behavior

if you do with curl or Chrome or whatever browser, the headers are always sorted as typed. but in Perl since headers are hash, the order is somehow not desired

Expected behavior

header Host is before header Accept for example.

Actual behavior

Host might be the last in Mojo::Headers::to_string

we can try to add "sorted_headers" as ArrayRef and fix the return part for 'sub names' in Mojo::Headers

if you need a patch, I can submit one. but first of all, please agree that's acceptable.

Thanks

@kraih
Copy link
Member

kraih commented Apr 12, 2022

What is the problem you are trying to solve?

@karenetheridge
Copy link
Contributor

references:

@kraih
Copy link
Member

kraih commented Apr 12, 2022

@karenetheridge Ordering headers that way is very expensive, we are not going to do that.

@karenetheridge
Copy link
Contributor

This could be enabled optionally (defaulting to off), so those who want the ordered headers at the expense of performance can have it.

@fayland
Copy link
Author

fayland commented Apr 13, 2022

one of my issue is that CloudFlare actually detects the order of headers. and return 403 forbidden if the order is not correct. (I can provider example if you need it)

can make it optional like has 'ordered_headers'; or pass { Host => 'a', Referer => 'b', __sorted => ['Host', 'Referer'] } as headers.

Thanks

@s1037989
Copy link
Contributor

Could this be done with a hook and therefore a plugin?

@jhthorsen
Copy link
Member

@fayland: In which case does it return a 403? I've been running my Mojo apps behind Cloudflare for many years and never experienced what you are saying.

@fayland
Copy link
Author

fayland commented Apr 13, 2022

CODE REMOVED

Thanks

@jhthorsen
Copy link
Member

@fayland: What I meant is that there's probably a some firewall rule (or another setting) in Cloudflare that has been enabled for this to happen, no..?

@fayland
Copy link
Author

fayland commented Apr 13, 2022

not sure. I have to overwrite Mojo::Headers::to_string to make it working right now. thinking it might be good if we can do a feature to make header sortable. if it's not acceptable, it's alright as well since I can do it with *Mojo::Headers::to_string

Thanks

@jhthorsen
Copy link
Member

jhthorsen commented Apr 13, 2022

It does sound a bit random though. Maybe just apply a role for this case?

# Make a custom to_string()
package Mojo::Headers::Role::Sorted;
use Mojo::Base -role, -signatures;
around to_string => sub ($next, $self) { ... };

# And then apply it to the user agent:
package main;
my $ua = Mojo::UserAgent->new;
$ua->on(start => sub ($ua, $tx) { $tx->req->headers->with_roles('+Sorted') });

@kraih
Copy link
Member

kraih commented Apr 18, 2022

I can replicate the problem with the test case, it fails randomly based on header order. What i find interesting is that even just alphabetical order of headers makes it pass.

@karenetheridge
Copy link
Contributor

does anyone know anyone at CloudFlare? This is such a flagrant violation of the RFCs, and it would actually take a lot of effort to fail like this; what on earth could the justification be for it??

@kraih
Copy link
Member

kraih commented Apr 18, 2022

Could be WebSocket specific, given the test case. Handshakes are easy to get wrong given their relationship to HTTP 1.1. But anyway, i'm happy to try alphabetical headers to make our generated HTTP messages more canonical. 64354fa

@karenetheridge
Copy link
Contributor

Alphabetical is not the "good practice" order though. https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html

@kraih
Copy link
Member

kraih commented Apr 18, 2022

Alphabetical is not the "good practice" order though. https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html

It's canonical and fast though.

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

5 participants