Project

General

Profile

Actions

Bug #84167

closed

Negation Bug in TYPO3 8.7 GuzzleHttp \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl()

Added by Felix Buenemann about 6 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Should have
Category:
Extbase
Target version:
-
Start date:
2018-03-07
Due date:
% Done:

100%

Estimated time:
1.00 h
TYPO3 Version:
8
PHP Version:
Tags:
Complexity:
no-brainer
Is Regression:
Yes
Sprint Focus:

Description

There's a negation bug in \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl() of TYPO3 8.7.10 that was introduced when the code was converted from curl to GuzzleHttp (see #70056).

The code replaced a check for $content === false in the curl code with !empty($content) in the GuzzleHttp code, which has the consequence of all reponses with a non-empty body being handled like an error and as a result missing the http_code and content_type field in the report.

The bug was introduced in revision 68a6da69a401e083dca1a9e7d368560a52ec0f68 check line 2111 in the old and 2055 in the new code.

The attached patch against the master branch should fix the problem and applies just fine to the 8.7 branch with a line offset.


Files


Related issues 3 (0 open3 closed)

Related to TYPO3 Core - Feature #70056: Use guzzleClosedGeorg Ringer2015-09-23

Actions
Related to TYPO3 Core - Bug #84170: Inconsistent lib value in \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl() since GuzzleHttp switchClosed2018-03-07

Actions
Related to TYPO3 Core - Bug #84173: TYPO3 8.7 GuzzleHttp \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl() returns content_type as array instead of stringClosed2018-03-07

Actions
Actions #1

Updated by Felix Buenemann about 6 years ago

  • Subject changed from Negation Bug in TYPO3 8.7 GuttzleHttp \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl() to Negation Bug in TYPO3 8.7 GuzzleHttp \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl()
Actions #2

Updated by Felix Buenemann about 6 years ago

Actions #3

Updated by Mathias Schreiber about 6 years ago

  • Status changed from New to Needs Feedback
  • Assignee set to Mathias Schreiber

Would you mind pushing a change?
I can do it too, but then all the credit goes to me while you found the issue in the first place

Actions #4

Updated by Felix Buenemann about 6 years ago

Mathias: Do you mean a PR against the TYPO3-CMS/core Github Repo or where should I direct it?

You can also use git apply <patchfile> which should retain the author and commit message (you would be listed as committer, me as the author).

Actions #5

Updated by Felix Buenemann about 6 years ago

Sorry, I meant git am not apply.

Actions #6

Updated by Mathias Schreiber about 6 years ago

I'll test whether that thing applies on master, too

Actions #7

Updated by Felix Buenemann about 6 years ago

It should, since the patch is against master from a few minutes ago. Offset in 8.7 is 237 lines or so.

Actions #8

Updated by Felix Buenemann about 6 years ago

  • Related to Bug #84170: Inconsistent lib value in \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl() since GuzzleHttp switch added
Actions #9

Updated by Mathias Schreiber about 6 years ago

The file does not apply.
I think this is mainly because it's taken from somewhere within the file structure, not from TYPO3's root dir.

In order to rule out any errors on my side:

git am /Volumes/Macintosh\ HD/Users/mathiasschreiber/Downloads/0001-BUGFIX-Negation-bug-in-getUrl-empty-body-check.patch
Applying: Negation bug in getUrl empty body check
error: Classes/Utility/GeneralUtility.php: does not exist in index
Patch failed at 0001 Negation bug in getUrl empty body check
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Tried both from the git root as well as from typo3/sysext/core

Actions #10

Updated by Felix Buenemann about 6 years ago

Oh, it was made against https://github.com/TYPO3-CMS/core I guess I got the wrong repo then.

Actions #11

Updated by Mathias Schreiber about 6 years ago

Try this: https://docs.typo3.org/typo3cms/ContributionWorkflowGuide/Setup/Git/Index.html

This takes roughly 5 minutes to walk through and helps in the manner that (in case questions come up) you're involved directly.
Plus it kinda saves me all the hassle right now :D

Actions #12

Updated by Felix Buenemann about 6 years ago

  • File deleted (0001-BUGFIX-Negation-bug-in-getUrl-empty-body-check.patch)
Actions #13

Updated by Felix Buenemann about 6 years ago

I updated the paths in the patch so that it applies against the main TYPO3.CMS git repo.

Actions #14

Updated by Felix Buenemann about 6 years ago

All right I'll look at the guide. I remember using Gerrit once some years ago and I found the interface very confusing to say the least.

Anyways I'll follow the guide and post an update once done.

Actions #15

Updated by Mathias Schreiber about 6 years ago

I can walk you through it via Voice if you like as well.
Hit me up on Slack when you find the time.

Actions #16

Updated by Gerrit Code Review about 6 years ago

  • Status changed from Needs Feedback to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56039

Actions #17

Updated by Mathias Schreiber about 6 years ago

Followup:
I applied the patch on master and added the necessary info to the commit message.
We'll need to change the commit message around a little bit... just follow the review mentioned in this ticket

Actions #18

Updated by Felix Buenemann about 6 years ago

Yeah, I noticed that git am drops all prefixes in brackets, that's counterproductive here. I'll follow the gerrit workflow directly for the next ticket.

Actions #19

Updated by Gerrit Code Review about 6 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56039

Actions #20

Updated by Gerrit Code Review about 6 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56039

Actions #21

Updated by Gerrit Code Review about 6 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56039

Actions #22

Updated by Felix Buenemann about 6 years ago

  • Related to Bug #84173: TYPO3 8.7 GuzzleHttp \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl() returns content_type as array instead of string added
Actions #23

Updated by Gerrit Code Review about 6 years ago

Patch set 1 for branch TYPO3_8-7 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56047

Actions #24

Updated by Felix Buenemann about 6 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #25

Updated by Benni Mack over 5 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF