Upstream sent duplicate header line

[madsi1m]

Hi team, When using discopower I notice the following in the logs when looking at disco.php. On both dev-master and latest release.

2024/08/21 03:28:41 [warn] 286#286: *4918 upstream sent duplicate header line: "Date: Wed, 21 Aug 2024 03:28:41 GMT", previous value: "Date: Wed, 21 Aug 2024 03:28:41 GMT", ignored while reading response header from upstream, client: x.x.x.x, server: x.x.x.x, request: "GET /simplesaml/module.php/discopower/disco.php?entityID=https%3A%2F%2Fx.x.x.x%2Fsimplesaml%2Fmodule.php%2Fsaml%2Fsp%2Fmetadata.php%2Fdefault-sp&return=https%3A%2F%2Fx.x.x.x%2Fsimplesaml%2Fmodule.php%2Fsaml%2Fsp%2FdiscoResponse%3FAuthID%3D_889a13be559c3295def3f9745d42df88a4d1fd33ca%253Ahttps%253A%252F%252Fx.x.x.x%252Fsimplesaml%252Fmodule.php%252Fsaml%252Fsp%252Flogin%252Fdefault-sp%253FReturnTo%253Dhttps%25253A%25252F%25252Fx.x.x.x%25252F%25253Fs%25253Dupload&returnIDParam=idpentityid HTTP/1.1", upstream: "fastcgi://127.0.0.1:9000", host: "x.x.x.x", referrer: "https://x.x.x.x"

The rest of SSP seems to not have this problem.

[tvdijen]

@thijskh I sort of remember us discussing a duplicate date-header, but I can't remember the details.. Do you?

Edit: I found simplesamlphp/simplesamlphp#1852 but that one was never merged and the reason for that wasn't mentioned in the PR.

[tvdijen]

@madsi1m Could you try if f8828bb changes anything in this weird behaviour?

[thijskh]

I think it was never merged because we considered it a corner case that was resolved by the refactoring already done in master...

[madsi1m]

Hi @tvdijen and @thijskh, thank you for the quick response!

@thijskh I sort of remember us discussing a duplicate date-header, but I can't remember the details.. Do you?

Edit: I found simplesamlphp/simplesamlphp#1852 but that one was never merged and the reason for that wasn't mentioned in the PR.

I manually added the date line, but needed to add it after parent constructor so it wouldn't error out and it did not work, I still get the duplicate header for Date.

@madsi1m Could you try if f8828bb changes anything in this weird behaviour?

I updated discopower to use the latest commit which includes this (I double checked) and again it did not work, I still get the duplicate header for Date.

I also increased the output_buffering on php and then disabled it and both times I still get the duplicate header for Date.

Happy to try other things as you think of them.

Our setup is very basic, php-fpm talking to nginx. The nginx log is what is reporting duplicate header line from php and it is only from discopower.

[tvdijen]

I think in addition to what @thijskh already said, it wasn't merged because it didn't work.
This issue is caused by our half-decent implementation of Symfony routing/controllers, and the problem with the proper implementation (already in place in our master-branch) is that it's a backwards-incompatible change. I kind of remember us agreeing that the duplicate date-header wasn't worth the hassle/effort of releasing a new major and update literally every module we have. It's a warning, it's annoying, but it doesn't harm anyone.

[tvdijen]

I think I may have found a workaround for this. The issue, if I remember correctly, is that we output a template ($t->send()) and capture that in a StreamedResponse and then output that again.

The fix involves changes in both this module and in SimpleSAMLphp, and it's technically a BC-break, but only for those who implement their own discovery service. I think with some proper upgrade notes it's okayish to release this in the next minor version of SSP.

@madsi1m Feel free to test this

Strike that.. I came to the conclusion that this cannot be done without a tremendous amount of work on SimpleSAMLphp.