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

MAX_NUM_COOKIES can not be overridden any more #39625

Open
2 of 5 tasks
yavery opened this issue Feb 12, 2025 · 7 comments
Open
2 of 5 tasks

MAX_NUM_COOKIES can not be overridden any more #39625

yavery opened this issue Feb 12, 2025 · 7 comments
Assignees
Labels
Reported on 2.4.6-p9 Indicates original Magento version for the Issue report. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it

Comments

@yavery
Copy link

yavery commented Feb 12, 2025

Preconditions and environment

  • Magento version

2.4.5-P11, 2.4.6-P9
I assume this change is included in all Feb 11 security releases

  • Additional Information

Comment for the code may be out-dated, I don't see any mention in the RFC document supporting MAX_NUM_COOKIES to be 50. On the contrary, it mentions "at least 50", which implies in most cases the number should be higher
Image

Steps to reproduce

upgrade to 2.4.5-P11, 2.4.6-P9

in previous versions:

namespace Magento\Framework\Stdlib\Cookie;
class PhpCookieManager implements CookieManagerInterface
{
    /**#@+
     * Constants for Cookie manager.
     * RFC 2109 - Page 15
     * http://www.ietf.org/rfc/rfc6265.txt
     */
    const MAX_NUM_COOKIES = 50;
    const MAX_COOKIE_SIZE = 4096;

and

        if ($numCookies > static::MAX_NUM_COOKIES) {
            $this->logger->warning(
                new Phrase('Unable to send the cookie. Maximum number of cookies would be exceeded.'),

after upgrade:

namespace Magento\Framework\Stdlib\Cookie;
class PhpCookieManager implements CookieManagerInterface
{
    /**#@+
     * Constants for Cookie manager.
     * RFC 2109 - Page 15
     * http://www.ietf.org/rfc/rfc6265.txt
     */
    private const MAX_NUM_COOKIES = 50;
    public const MAX_COOKIE_SIZE = 4096;

and

        if ($numCookies > self::MAX_NUM_COOKIES) {
            $this->logger->warning(
                new Phrase('Unable to send the cookie. Maximum number of cookies would be exceeded.'),
        }

Expected result

Magento codebase use more than 50 cookies, Application needs to override the MAX_NUM_COOKIES

Previously, this can be done through an overriding via preference:

namespace Example\Framework\Stdlib\Cookie;
class PhpCookieManager extends \Magento\Framework\Stdlib\Cookie\PhpCookieManager
{
    const MAX_NUM_COOKIES = 1000;
}

Actual result

After upgrade, every call to server would generate a new log entry:

self::MAX_NUM_COOKIES only access the the private const MAX_NUM_COOKIES

[2025-02-11T22:41:45.604288+00:00] report.WARNING: Unable to send the cookie. Maximum number of cookies would be exceeded. {"cookies":["BVBRANDID","BVBRANDSID","PHPSESSID","UsableNetAssistive","X-Magento-Vary","__attentive_cco","__attentive_ceid","__attentive_domain","__attentive_dv","__attentive_id","__attentive_pv","__attentive_session_id","__attentive_ss_referrer","__mmapiwsid","attn","_dy_df_geo","_dy_geo","_dy_soct","_dy_toffset","_dycnst","_dycst","_dyid","_dyid_server","_dyjsession","_fbp","_ga","_ga_1YTZ6HDQ0N","_gcl_au","_gid","_pin_unauth","_rdt_uuid","_tt_enable_cookie","_ttp","_uetsid","_uetvid","amp_e133e6","amp_e133e6_example_com","apt_pixel","attntv_mstore_email","avy_ic","avy_it","btid","consumerId","crl8_fpcuid","customerType","dy_fs_page","form_key","intent_audience","mage-cache-sessid","mage-cache-storage","mage-cache-storage-section-invalidation","mage-messages","osano_consentmanager","osano_consentmanager_uuid","private_content_version","product_data_storage","recently_compared_product","recently_compared_product_previous","recently_viewed_product","recently_viewed_product_previous","section_data_clean","section_data_ids","stencilUser","v2_example","v2_example-sandbox","wwjH7clZ8E5v5BsgCqQ5zHYf9OlZ8dCB"],"user-agent":"Amazon CloudFront"} []

Additional information

Judging by new release adding private in front of const MAX_NUM_COOKIES, is Magento trying to make it hard for application to change MAX_NUM_COOKIES and expressing a strong recommendation on this

Release note

No response

Triage and priority

  • Severity: S0 - Affects critical data or functionality and leaves users without workaround.
  • Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
  • Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
  • Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
  • Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.
Copy link

m2-assistant bot commented Feb 12, 2025

Hi @yavery. Thank you for your report.
To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce.


Join Magento Community Engineering Slack and ask your questions in #github channel.
⚠️ According to the Magento Contribution requirements, all issues must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.
🕙 You can find the schedule on the Magento Community Calendar page.
📞 The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

@engcom-Bravo engcom-Bravo added the Reported on 2.4.6-p9 Indicates original Magento version for the Issue report. label Feb 13, 2025
@github-project-automation github-project-automation bot moved this to Ready for Confirmation in Issue Confirmation and Triage Board Feb 13, 2025
@hostep
Copy link
Contributor

hostep commented Feb 13, 2025

I assume this was done on purpose @nathanjosiah?

There is however a quality patch to increase the max number of cookies allowed from 50 to 200: MDVA-12304, however, it's not compatible yet with the latest security releases. Follow magento/quality-patches#133 to see how it progresses.

If you really need an urgent fix, I suggest you create your own patch and apply it to your shop, it shouldn't be hard.

@engcom-Bravo engcom-Bravo added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Feb 13, 2025
@engcom-Hotel engcom-Hotel self-assigned this Feb 20, 2025
Copy link

m2-assistant bot commented Feb 20, 2025

Hi @engcom-Hotel. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

  • 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
  • 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue.
  • 3. Add Area: XXXXX label to the ticket, indicating the functional areas it may be related to.
  • 4. Verify that the issue is reproducible on 2.4-develop branch
    Details- If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
  • 5. Add label Issue: Confirmed once verification is complete.
  • 6. Make sure that automatic system confirms that report has been added to the backlog.

@engcom-Hotel
Copy link
Contributor

Hello @yavery,

Thanks for the report and collaboration!

As per the discussion internally, this issue has been fixed in the scope of JIRA https://jira.corp.adobe.com/browse/AC-13918, and planned to release in April.

Hence closing this issue.

@hostep
Copy link
Contributor

hostep commented Feb 20, 2025

@engcom-Hotel: please do not close issues when the fix hasn't been pushed to the open source repository here on github. Thanks!

@engcom-Hotel
Copy link
Contributor

Sure @hostep, let me reopen it and move it to On Hold bucket.

Thanks

@yavery
Copy link
Author

yavery commented Feb 22, 2025

by the way, the jira link is inaccessible for invalid ssl

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Reported on 2.4.6-p9 Indicates original Magento version for the Issue report. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
Development

No branches or pull requests

4 participants