Skip to content

Fix #73630: Built-in Weberver - overwrite $_SERVER['request_uri'] #7207

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

Closed
wants to merge 5 commits into from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Jun 29, 2021

The built-in Webserver's on_path, on_query_string and on_url
callbacks may be called multiple times from the parser; we must not
simply replace the old values, but need to concatenate the new values
instead.

This appears to be tricky for on_path due to the path normalization,
so we fail if the function is called again.

The built-in Webserver's `on_path`, `on_query_string` and `on_url`
callbacks may be called multiple times from the parser; we must not
simply replace the old values, but need to concatenate the new values
instead.

This appears to be tricky for `on_path` due to the path normalization,
so we fail if the function is called again.
@cmb69 cmb69 added the Bug label Jun 29, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #7207 (98a21d1) into PHP-7.4 (51e2015) will increase coverage by 0.02%.
The diff coverage is 86.20%.

❗ Current head 98a21d1 differs from pull request most recent head b4dda02. Consider uploading reports for the commit b4dda02 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           PHP-7.4    #7207      +/-   ##
===========================================
+ Coverage    72.99%   73.02%   +0.02%     
===========================================
  Files          803      803              
  Lines       283801   283937     +136     
===========================================
+ Hits        207155   207338     +183     
+ Misses       76646    76599      -47     
Impacted Files Coverage Δ
ext/gd/gd.c 73.31% <ø> (ø)
ext/gd/libgd/gd_crop.c 80.76% <ø> (ø)
ext/gd/libgd/gd_interpolation.c 72.64% <ø> (ø)
ext/gd/libgd/gd_wbmp.c 71.66% <ø> (ø)
ext/opcache/Optimizer/zend_func_info.c 90.76% <ø> (ø)
ext/openssl/xp_ssl.c 74.56% <ø> (ø)
main/streams/plain_wrapper.c 67.09% <ø> (ø)
sapi/fpm/fpm/events/port.c 100.00% <ø> (ø)
sapi/fpm/fpm/fpm_atomic.h 57.14% <ø> (ø)
sapi/fpm/fpm/fpm_unix.c 31.52% <0.00%> (ø)
... and 56 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6130dd...b4dda02. Read the comment docs.

The total length of the HTTP headers is already restricted to at most
`PHP_HTTP_MAX_HEADER_SIZE` bytes, so no integer overflow can occur in
our calculations.
cmb69 added 3 commits June 30, 2021 11:56
The built-in Webserver logs errors during request parsing to stderr,
but this is ignored by the php_cli_server framework, and apparently the
Webserver does not send a resonse at all in such cases (instead of an
4xx).  Thus we can only check that a request with an overly long path
fails.
@cmb69 cmb69 closed this in d7db570 Jun 30, 2021
@cmb69 cmb69 deleted the cmb/73630 branch June 30, 2021 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants