erlyaws / yaws

Yaws webserver
https://erlyaws.github.io
BSD 3-Clause "New" or "Revised" License
1.28k stars 267 forks source link

Handle requests with multiple Accept headers #314

Closed avtobiff closed 5 years ago

avtobiff commented 6 years ago

RFC 7230 HTTP/1.1 Message Syntax and Routing Section 3.2.2. Field Order states that a server MAY combine multiple header fields with the same name into a comma-separated list [i.e. #(values)], withouth changing the request semantics.

This change transforms multiple Accept headers in a request into such a comma-separated list.

vinoski commented 6 years ago

Note that all Travis tests are failing for this change.

avtobiff commented 6 years ago

Note that all Travis tests are failing for this change.

Everything works for me locally (except the deflate_SUITE see #315).

See e.g. https://travis-ci.org/klacke/yaws/jobs/306288909#L1572 It seems Travis runs great until it hits the revproxy_SUITE, where it fails with badbind:

  revproxy_SUITE...

2017-11-23 12:21:10 crash_report        

    initial_call:     pid:     registered_name:     error_info:     ancestors:     messages:     links:     dictionary:     trap_exit:     status:     heap_size:     stack_size:     reductions: 2017-11-23 12:21:10 ** Generic server ~p terminating 

** Last message in was ~p~n** When Server state == ~p~n** Reason for termination == ~n** ~p~n

                2017-11-23 12:21:10 crash_report        2017-11-23 12:21:10 Error in process ~p with exit value:~n~p~n

        2017-11-23 12:21:10 crash_report        

    initial_call:     pid:     registered_name:     error_info:     ancestors:     messages:     links:     dictionary:     trap_exit:     status:     heap_size:     stack_size:     reductions: 2017-11-23 12:21:10 Error in process ~p with exit value:~n~p~n

        Kernel pid terminated (application_controller) ({application_start_failure,yaws,{{shutdown,{failed_to_start_child,yaws_server,{badbind,[{yaws_server,start_group,2,[{file,"yaws_server.erl"},{line,261}]

Crash dump is being written to: erl_crash.dump...done

============================================================================

 see /home/travis/build/klacke/yaws/testsuite/logs/yaws.log for details

 common_test result: /home/travis/build/klacke/yaws/testsuite/logs/ct_run.ct@travis-job-klacke-yaws-306288909.2017-11-23_12.19.42/index.html

============================================================================

make[2]: *** [common_test] Error 1

make[2]: Leaving directory `/home/travis/build/klacke/yaws/testsuite'

make[1]: *** [check-am] Error 2

make[1]: Leaving directory `/home/travis/build/klacke/yaws/testsuite'

make: *** [check-recursive] Error 1

The command "make check -j4 V=1" exited with 2.
after_script

0.01s$ ./scripts/upload-travis-build-artifacts

ERROR; missing github token

Done. Your build exited with 1.

I would be surprised if this was due to my pull request.

I'm happy to help debug the CI issue!

capflam commented 6 years ago

There is definitely an issue with the testsuite on Travis. I'm unable to reproduce the problem on my own. So it is not really easy to fix it. But, I agree with @avtobiff, this is not a problem with the pull request.

BTW, I will try to review the PR very soon. Thanks !

etnt commented 5 years ago

Hi guys, what is the status of this pull request?

vinoski commented 5 years ago

I haven't seen any replies, so I'll take a look.

vinoski commented 5 years ago

I reran the Travis-CI tests and everything passed. Now reviewing the changes.

vinoski commented 5 years ago

I manually rebased this change to master, so closing this now.

Thanks for the contribution, and apologies for the delay.

etnt commented 5 years ago

Thanks a lot!