get csrf token in request and test for prefix 'http-' in csrf token header #501
Conversation
|
Hello, hier are my test results: routes: result: |
|
Hello, okay, so I think you deleted your request for a test. ~ Marius |
|
I was supposed to push the changes to this pull request, but i did something wrong and it created a new PR. A little messy, but it's all merged now. Nice work - added some more feature to your work 👍 I think i'll change the Something like this in the foreach ($_SERVER as $key => $value) {
$this->headers[strtolower($key)] = $value;
$this->headers[str_replace(['_', 'http-'], ['-', ''], strtolower($key))] = $value;
}It always keep a copy of the original header. But then we don't have to check if the |
|
Yes this would be easier, but I think it can lead to some problems. Your idea with keeping the original headers is not bad, but then we have to store the same data two times. getHeader(string $name) -> looks over both arrays for the header name. The parameter name gets processed by tolowercase' replace http prefix, _ replaced with -. getServerHeader(string $name) -> same process for the parameter and this function look just over the server headers (non http prefix). Maby think more about the function name. What do you think @skipperbent ? ~ Marius |
|
You're right i can see my idea causing some problems. Two methods is more code. Both in the project and for the user. We want the project to do the work for us 99% percent of the time without us having to juggle between multiple methods ( Found a solution that might do the trick without storing both versions of the headers. This way it can store all the original headers and when calling Example: /**
* Get header value by name
*
* @param string $name Name of the header.
* @param string|null $defaultValue Value to be returned if header is not found.
* @param bool $tryParse When enabled the method will try to find the header from both from client (http) and server-side variants, if the header is not found.
*
* @return string|null
*/
public function getHeader(string $name, $defaultValue = null, $tryParse = true): ?string
{
$name = strtolower($name);
$header = $this->headers[$name] ?? null;
if ($tryParse === true && $header === null) {
if (strpos($name, 'http-') === 1) {
// Trying to find client header variant which was not found, searching for header variant without http- prefix.
$header = $this->headers[str_replace('http-', '', $name)] ?? null;
} else {
// Trying to find server variant which was not found, searching for client variant with http- prefix.
$header = $this->headers['http-' . $name] ?? null;
}
}
return $header ?? $defaultValue;
} |
|
Hmm.. that code in issue #491 seems to do exactly the same as the code posted above. foreach ($_SERVER as $key => $value) {
$this->headers[strtolower($key)] = $value;
$this->headers[str_replace(['_', 'http-'], ['-', ''], strtolower($key))] = $value;
}But as you said, if you for some unknown reason needed both of them, only one of them will be available as the http variant will overwrite the standard one non http one. Not sure what the chances are of people needing both though. |
I would do it like you posted it here. The possibility is small but it is possible ^^ |
|
I agree - at some point we need this in some weird use case 2 years later and then it will be confusing as of why it overwrites the header. Better to make it work properly from the beginning. Thanks for your feedback, I really appreciate it :) |
|
And nicely spottet with that strpos :) thanks |
Hello Simon,
as @mmdm95 mentioned in #491 in some requests php recieve the
x-csrf-tokenwith ahttp-prefix.I also have to get the header using the
http-preifx. I'm using the following method to set the header in js:jqxhr.setRequestHeader('X-CSRF-TOKEN', csrf_token);and recieve it in the
CsrfVerifierusing:$request->getHeader('HTTP-X-CSRF-TOKEN')In this commit I added a function to the
Requestthat returns thex-csrf-tokenheader with and without thehttp-prefix.I also added this to the
BaseCsrfVerifierin thehandlefunction.It is explained in this stackoverflow post:
I'm not really sure why some people can recieve the header without the
http-prefix, but I think it is better to add support.I only added the
http-prefix option as alternative and the non prefix one as original, because I think the header shoud not have thishttp-prefix.~ Marius
Update:
I also added the
patchmethod to theBaseCsrfVerifier::handlefunction.Yes, the patch method does not contain any post parameters, but it will instantly fallback to the
defaultValueand by that access the header.~ Marius