pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/guzzle/guzzle/pull/2293

om/assets/global-b40ec823a1a6a1af.css" /> [8.0] Update `EasyHandle` in order to avoid bad writes of properties related to the handle by phansys · Pull Request #2293 · guzzle/guzzle · GitHub
Skip to content

[8.0] Update EasyHandle in order to avoid bad writes of properties related to the handle#2293

Open
phansys wants to merge 1 commit intoguzzle:8.0from
phansys:easy_handle
Open

[8.0] Update EasyHandle in order to avoid bad writes of properties related to the handle#2293
phansys wants to merge 1 commit intoguzzle:8.0from
phansys:easy_handle

Conversation

@phansys
Copy link
Copy Markdown
Contributor

@phansys phansys commented Apr 27, 2019 •
edited
Loading

Q A
Branch master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

With these changes, setting fake values like this can be prevented.

@phansys phansys force-pushed the easy_handle branch 3 times, most recently from 9ff0a64 to a1d7847 Compare April 29, 2019 14:52
@Nyholm
Copy link
Copy Markdown
Member

Nyholm commented Oct 18, 2019

Thank you for the PR.
I would like you to clarify one thing for me: From whom are you preventing to set fake values?

@phansys
Copy link
Copy Markdown
Contributor Author

phansys commented Oct 18, 2019

The prevention is done in order to avoid the object to accept a set of inconsistent values, regardless who is writing them. In my specific use case, the situation was caused from a 3rd party library using Guzzle, but I don't have enough details since I think the problem was related to #2292.

@Nyholm Nyholm added this to the 8.0.0 milestone Dec 8, 2019
@phansys phansys force-pushed the easy_handle branch 2 times, most recently from 0975ccd to c782faf Compare January 6, 2020 17:53
GrahamCampbell
GrahamCampbell reviewed Jan 6, 2020
View reviewed changes
Comment thread src/Handler/CurlFactory.php Outdated
// Retry when nothing is present or when curl failed to rewind.
if (empty($easy->options['_err_message'])
&& (!$easy->errno || $easy->errno == 65)
&& (!$easy->errno || $easy->errno == /* CURLE_SEND_FAIL_REWIND */ 65)
Copy link
Copy Markdown
Member

@GrahamCampbell GrahamCampbell Jan 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is this constant not set? Was added at least 10 years ago, wasn't it?

Copy link
Copy Markdown
Contributor Author

@phansys phansys Jan 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. See php/php-src#4079.

Copy link
Copy Markdown
Member

@GrahamCampbell GrahamCampbell Jan 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. They never made it into PHP-land. ;)

@phansys phansys force-pushed the easy_handle branch 2 times, most recently from 7f8c70a to 0998eab Compare January 6, 2020 22:09
@stale
Copy link
Copy Markdown

stale Bot commented Sep 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 2 weeks if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the lifecycle/stale No activity for a long time label Sep 25, 2020
@Nyholm Nyholm added the lifecycle/keep-open Issues that should not be closed label Sep 25, 2020
@stale stale Bot removed the lifecycle/stale No activity for a long time label Sep 25, 2020
@phansys phansys force-pushed the easy_handle branch 4 times, most recently from 8e43093 to dd0ea36 Compare June 9, 2021 00:25
GrahamCampbell
GrahamCampbell requested changes Jun 9, 2021
View reviewed changes
Comment thread src/Handler/EasyHandle.php Outdated
@phansys phansys force-pushed the easy_handle branch 6 times, most recently from 2a25470 to 36c07f3 Compare June 11, 2021 00:06
@GrahamCampbell GrahamCampbell removed this from the 8.0.0 milestone Mar 31, 2024
@GrahamCampbell GrahamCampbell changed the title Update EasyHandle in order to avoid bad writes of properties related to the handle [8.0] Update EasyHandle in order to avoid bad writes of properties related to the handle Mar 31, 2024
@GrahamCampbell GrahamCampbell changed the base branch from 7.5 to 8.0 March 31, 2024 19:45
@GrahamCampbell
Copy link
Copy Markdown
Member

GrahamCampbell commented Mar 31, 2024

@phansys are you able to resolve the merge conflicts please?

@phansys phansys force-pushed the easy_handle branch 6 times, most recently from c14b88a to 229317f Compare April 1, 2024 12:41
@phansys
Copy link
Copy Markdown
Contributor Author

phansys commented Apr 1, 2024

@phansys are you able to resolve the merge conflicts please?

Since this PR is almost 5 years old, I guess I must also update the behavior on some checks, since the added support for PHP >= 8 in the target branch also changed some return values in CURL functions (like resource => CurlHandle).

@phansys phansys force-pushed the easy_handle branch 2 times, most recently from ed9dfc1 to 2245ac1 Compare April 1, 2024 13:48
mfn
mfn reviewed Apr 1, 2024
View reviewed changes
Comment thread src/Handler/EasyHandle.php Outdated
}

/**
* @throws \LogicException
Copy link
Copy Markdown
Contributor

@mfn mfn Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does it throw LogicException in __unset?

Copy link
Copy Markdown
Contributor Author

@phansys phansys Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This exception isn't thrown in the current implementation. The docblock was updated, thank you.

@phansys phansys force-pushed the easy_handle branch 2 times, most recently from 267b28b to 0e8c2ca Compare April 1, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GrahamCampbell GrahamCampbell GrahamCampbell requested changes

+2 more reviewers

@mfn mfn mfn left review comments

@YasinAshorian YasinAshorian YasinAshorian approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

lifecycle/keep-open Issues that should not be closed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@phansys @Nyholm @GrahamCampbell @mfn @YasinAshorian
pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.




Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy