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

Fix bugs #75120 and #69625 #3226

Closed
wants to merge 5,815 commits into from

Conversation

@Felixoid
Copy link

commented Apr 27, 2018

On empty SCRIPT_FILENAME and/or REQUEST_METHOD return HTTP 500

I reused patch from PR #1270 and tested it.
php-fpm now return "500 method not found" instead of "200 with an empty body".

weltling and others added some commits Mar 12, 2018

Merge branch 'PHP-7.2'
* PHP-7.2:
  Use literal as format
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  Fix heap use after free
Merge branch 'PHP-7.2'
* PHP-7.2:
  Fix heap use after free
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  Fixed bug #76085 (Segmentation fault in buildFromIterator when directory name contains a \n)
Merge branch 'PHP-7.2'
* PHP-7.2:
  Fixed bug #76085 (Segmentation fault in buildFromIterator when directory name contains a \n)
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  Forgot NEWS
Merge branch 'PHP-7.2'
* PHP-7.2:
  Forgot NEWs
  Forgot NEWS
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  More accurate symbolic constraints oferflow/unserflow handling (better fix for bug #76074).
Merge branch 'PHP-7.2'
* PHP-7.2:
  More accurate symbolic constraints oferflow/unserflow handling (better fix for bug #76074).
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  Revert "More accurate symbolic constraints oferflow/unserflow handling (better fix for bug #76074)."
Merge branch 'PHP-7.2'
* PHP-7.2:
  Revert "More accurate symbolic constraints oferflow/unserflow handling (better fix for bug #76074)."
Merge branch 'PHP-7.2'
* PHP-7.2:
  next is 7.2.5
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  Fix #76088: ODBC functions are not available by default on Windows
Merge branch 'PHP-7.2'
* PHP-7.2:
  Fix #76088: ODBC functions are not available by default on Windows
@nikic

This comment has been minimized.

Copy link
Member

commented on 0d6da03 Mar 13, 2018

The current tokenizer output is intentional, we need the tokens to look as if no error occurred in order to properly support parsing with error recovery and similar. It will be possible to work around this by manually parsing the T_ERROR tokens and restoring their proper token IDs, but it will be a pain.

Ref: d91aad5 returned T_ERROR from token_get_all and a49ce7b disabled again shortly afterwards.

This comment has been minimized.

Copy link
Member Author

replied Mar 13, 2018

OK. I'll think how to handle this and restore behaviour for tokenizer (today evening or tomorrow).

This comment has been minimized.

Copy link
Member Author

replied Mar 13, 2018

I've reverted this and applied another scanner optimization patch.

dstogov and others added some commits Mar 13, 2018

Revert "Handle scanner error in first place (don't hide them from ext…
…/tokenizer) and cheaper whitespace handlig."

This reverts commit 0d6da03.
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  Fix #74139: mail.add_x_header default inconsistent with docs [ci skip]
Merge branch 'PHP-7.2'
* PHP-7.2:
  Fix #74139: mail.add_x_header default inconsistent with docs [ci skip]
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  bump versions
Merge branch 'PHP-7.2'
* PHP-7.2:
  bump versions
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  Fixed use-after-free
Merge branch 'PHP-7.2'
* PHP-7.2:
  Fixed use-after-free

weltling and others added some commits Apr 24, 2018

Merge branch 'PHP-7.2'
* PHP-7.2:
  [ci skip] Update NEWS
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  7.0.31 next
Merge branch 'PHP-7.2'
* PHP-7.2:
  7.0.31 next
Merge branch 'PHP-7.2'
* PHP-7.2:
  update NEWS for 7.2.5
Update NEWS
Add whitespace in NEWS file
Revamp pcre config for build with external lib
- support multiarch in addition to the usual lib path
- fix symbol checks
- fix jit availability check
- exit early on unsupported version
Switch to more robust config for external pcre2
For the standard layout the first option is pkg-config. Otherwise,
pcre2-config is used, which is produced by a manual installation into a
prefix. This removes the most of the hackish pieces like checking for
the lib filenames and parsing headers.
Avoid using _N in SSE code
The _N constant is already defined in OpenSSL. Instead use some
more explicit variable names.
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  Fix memory leak when phpdbg fails to start up
Merge branch 'PHP-7.2'
* PHP-7.2:
  Fix memory leak when phpdbg fails to start up
Merge branch 'PHP-7.1' into PHP-7.2
* PHP-7.1:
  Allocate default ini values into persistent memory
Merge branch 'PHP-7.2'
* PHP-7.2:
  Allocate default ini values into persistent memory
Fix bugs #75120 and #69625
On empty SCRIPT_FILENAME and/or REQUEST_METHOD return HTTP 500

@Felixoid Felixoid changed the base branch from master to PHP-7.1 Apr 29, 2018

@Felixoid Felixoid closed this Apr 29, 2018

@Felixoid

This comment has been minimized.

Copy link
Author

commented Apr 29, 2018

I read CONTRIBUTING.md, will make PR into PHP-7.1

@Felixoid Felixoid changed the base branch from PHP-7.1 to master Apr 29, 2018

@dktapps

This comment has been minimized.

Copy link
Contributor

commented on Zend/zend_hash.c in d7f2dc4 Jul 3, 2018

This causes unexpected behaviour when moving forward on a HT which does not have an index 0 (if it was deleted for example, like this code). If moving forward from index 0 (non existing) where the first existing position is 1, move_forward will now move the position to 2 instead of the expected 1, skipping 1 completely (because of the below idx++). All other cases behave as expected.

I don't know if that's behaviour you might want to consider fixing, or if such behaviour would have been considered undefined.

@Majkl578

This comment has been minimized.

Copy link
Contributor

commented on ext/standard/tests/array/array_unshift_empty.phpt in f7f4864 Aug 31, 2018

@timurib Can you please explain the reasoning behind this decision? This clearly doesn't look correct. I understand the reasoning behind ... (when empty), but if that means it also allows array_unshift with one argument, I think that's too high price for such an edge case.

This comment has been minimized.

Copy link
Contributor

replied Aug 31, 2018

@Majkl578 I don't think this is a real issue. If array_unshift() is called with a single parameter, that's a NOP now. Quite similar to calling count() and ignoring the return value. Note also that array_merge() can be called with a single argument, too. Anyhow, if you don't like this change, I suggest to discuss this on internals@.

This comment has been minimized.

Copy link
Contributor

replied Aug 31, 2018

This comment has been minimized.

Copy link
Contributor Author

replied Sep 3, 2018

@Majkl578 Imo, this case is like addition of zero: just nothing changes. And it actually worked this way always (https://3v4l.org/VsrhI ), the patch just removes the warning message. I explained more details in the PR description.
Sorry for late answer.

This comment has been minimized.

Copy link
Contributor Author

replied Sep 3, 2018

I supposed this change is obvious and didn't expect that it raised doubts. Maybe it really need additional discussion, the patch can be reverted before release of 7.3 (?)

This comment has been minimized.

Copy link

replied Sep 3, 2018

Where should I post an issue to discuss it with php internals?

This comment has been minimized.

Copy link
Contributor

replied Sep 3, 2018

@Pierstoval Write to internals at our mailing list server (lists.php.net). If you want to subscribe to the mailing list (not strictly necessary), send an email to internals-subscribe at the same server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.