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.

Sign up for GitHub

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

Jump to bottom

allow _resolveArguments to do sequential lookups #463

Closed
wants to merge 1 commit into from
Closed

allow _resolveArguments to do sequential lookups #463

wants to merge 1 commit into from

Conversation

danslo
Copy link
Contributor

@danslo danslo commented Jan 7, 2014

When resolving the arguments of PHP's internal classes, we cannot be
sure about argument names. In many places of the PHP core, documentation
differs from the implementation.

A good example is the FilterIterator which triggered this bugreport:

This can cause problems with PHP runtimes (read: HHVM) that properly
implement the documented specs. See the following HHVM issue for more
information:
facebook/hhvm#1492

The attached patch keeps existing functionality working, but adds an
additional fallback to sequentially based argument lookups. This is
demonstrated in the change to Menu.php.

@danslo
allow _resolveArguments to do sequential lookups
a517275 
When resolving the arguments of PHP's internal classes, we cannot be
sure about argument names. In many places of the PHP core, documentation
differs from the implementation.

A good example is the FilterIterator which triggered this bugreport:
- The argument name is documented as $it:
https://github.com/php/php-src/blob/PHP-5.6/ext/spl/internal/filteriterator.inc#L35
- But it is implemented as $iterator:
https://github.com/php/php-src/blob/PHP-5.6/ext/spl/spl_iterators.c#L2305

This can cause problems with PHP runtimes (read: HHVM) that properly
implement the documented specs. See the following HHVM issue for more
information:
facebook/hhvm#1492

The attached patch keeps existing functionality working, but adds an
additional fallback to sequentially based argument lookups. This is
demonstrated in the change to Menu.php.
@verklov
Copy link
Contributor

verklov commented Feb 13, 2014

@danslo , thank you for contributing to Magento 2! The issue has been created in the internal tracker and placed to the product backlog. When the team will have the contributed code and the suggested workaround analyzed, we will come up with the solution and update this issue.

@verklov verklov self-assigned this Feb 13, 2014
@verklov
Copy link
Contributor

verklov commented Feb 18, 2014

@danslo , the team has fixed the issue you reported. Please expect the fix to be released in the nearest code update. We will close the issue once the code is released.

magento-team added a commit that referenced this pull request Feb 28, 2014
@magento-team
2.0.0.0-dev67
b1cebfa 
* GitHub requests:
  * [#235](#235) -- Translation escaping
  * [#463](#463) -- allow _resolveArguments to do sequential lookups
* Fixed bugs:
  * Fixed an issue where nonexistent store views flat tables cleanuper dropped the catalog_category_flat_cl table
  * Fixed an issue where the Product Flat Data indexer used the helpers logic instead of the Flat State logic
  * Fixed an issue where an exception was thrown when applying a coupon code
  * Fixed an issue where a Shopping Cart Price Rule was applied to the wrong products
  * Fixed an issue with the broken Related Orders link on the Recurring Profile page
  * Fixed an issue with CMS pages preview not working
  * Fixed an issue with a sales report for a store view returning wrong result
  * Fixed an issue where shipping did not work for orders containing only bundle products
  * Fixed an issue where a custom not found page action did not work
  * Fixed an issue where user configuration for a shopping cart rule to stop further rules processing was ignored
* Modularity improvements:
  * Resolved dependencies of the Sales module on the RecurringProfile module
  * Resolved dependencies of the Email Templates functionality on application modules
  * Lib-only dependent components of the Core module moved to library
  * CSS URL resolving logic moved from the publisher to a separate CSS pre-processor
  * Refactored the View publisher
* Customer Service usage:
  * Refactored the Sales module to use Customer service layer
  * Refactored the Checkout module to use Customer service layer
* Updated various PHPDoc with the parameter and return types
@verklov
Copy link
Contributor

verklov commented Feb 28, 2014

@danslo, we have just released the dev67 code update with your contribution taken into account. We are closing this issue. Thank you for your contribution!

@verklov verklov closed this Feb 28, 2014
@danslo danslo deleted the factory-resolve-arguments branch February 28, 2014 21:53
@danslo
Copy link
Contributor Author

danslo commented Feb 28, 2014

Thanks!

magento-team added a commit that referenced this pull request Mar 18, 2014
@magento-team
2.0.0.0-dev69
2defd65 
* Themes update:
  * LESS styles library added in pub/lib/css/
  * A new Blank theme set as default
* GitHub requests:
  * [#491](#491) -- Fixed bug, incorrect auto-generation Category URL for some groups of symbols (idish, cirrilic, , , and other).
  * [#480](#480) -- Fixing a bug for loading config from local.xml
  * [#472](#472) -- Params passed in pub/index.php being overwritten
  * [#461](#461) -- Use translates for Quote\Address\Total\Shipping
  * [#235](#235) -- Translation escaping
  * [#463](#463) -- allow _resolveArguments to do sequential lookups
  * [#499](#499) Deleted unclosed comment in calendar.css
* Fixed bugs:
  * Fixed a fatal error that occurred with a dependency in pub/errors/report.php
  * Fixed an issue where code coverage failed for Magento\SalesRule\Model\Rule\Action\Discount\CartFixedTest
  * Fixed an issue where PayPal Express Checkout redirected to the PayPal site even though the Allow Guest Checkout option was set to 'No'
  * Fixed an issue where invalid password reset link was sent when resetting customer password from the backend
  * Fixed an issue where it was not possible to download a previously created backup
  * Fixed a security issue with possibility of a XSS injection in the Integration re-authorization flow
  * Fixed an issue where Billing Agreement cancellation from the backend did not work
  * Fixed an issue with the debug section in the developer settings
  * Fixed the unreliable implementation of the fetching authorization header via SOAP
  * Fixed issues with WSDL generation error reporting
  * Fixed an issue with incorrect order of the Recurring Profile tab in Account Customer on the frontend
  * Fixed an issue when the information about a custom option of the 'File' type was not displayed correctly on the recurring profile page
  * Fixed an issue with editing Product template
  * Fixed an issue with duplicated shipping method options during checkout
  * Fixed an issue where flat indexers were re-indexed in shell when they were disabled
  * Fixed an issue where adding a wrong/nonexistent SKU using 'Order by SKU' from My Account caused a fatal error
  * Fixed an issue with the JS/CSS merging functionality
  * Fixed an issue with static view files publication tool used for the 'production' mode
* Modularity improvements:
  * Removed the deprecated GoogleCheckout functionality
  * Removed all dependencies on the RecurringPayment module
  * Removed the Sales module dependencies on Customer models/blocks
  * Renamed the RecurringProfile module to RecurringPayment
  * Resolved dependencies between the Email Templates functionality and other modules
  * Moved Core module lib-only depended components to library
  * Moved CSS URL resolving logic from publisher to the separate CSS pre-processor
  * Re-factored the View publisher
* Framework improvements:
  * Added restrictions on the data populated to the Service Data Object
  * Renamed Data Transfer Object to Service Data Object
  * Updated the view files population tool to support LESS
* Customer Service usage:
  * Refactored the Tax module to use Customer service layer
  * Refactored Customer module Adminhtml internal controllers and helper to use Customer services
  * Added and updated the Customer service APIs
  * Exposed Customer services as REST APIs
* Indexer implementation:
  * Implemented a new optimized Product Price Indexer
* Updated various PHPDoc with the parameter and return types
vpelipenko added a commit that referenced this pull request Jul 15, 2015
@vpelipenko
Merge pull request #463 from magento-south/BUGS
06beed0 
[South] Code coverage
mmansoor-magento pushed a commit that referenced this pull request Oct 6, 2016
Merge pull request #463 from magento-falcons/MAGETWO-59264
6674762 
[Falcons] Bug fixes for Composer, Cache, Catalog
magento-devops-reposync-svc pushed a commit that referenced this pull request May 24, 2022
@andimov
Merge pull request #463 from magento-performance/CABPI-427
6508982 
CABPI-427: [BUG] User Identity Verification does not work on New Integration page with enabled AdminIMS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@verklov verklov

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danslo @verklov