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

Advanced Targeting Conditions: pum_alm_pages_viewed cookie size everbloat #1009

Open
lkraav opened this issue Sep 22, 2022 · 4 comments
Open
Labels

Comments

@lkraav
Copy link

lkraav commented Sep 22, 2022

Describe the bug

ATC extension is able to bloat pum_alm_pages_viewed cookie so large, server-configured cookie size limits are hit, and 400 Bad Request errors block further visits, until respective cookie is deleted.

It's probably not a good idea to start increasing server request header size limits to arbitrary numbers for this.

Site information

Popup Maker version: 1.16.8
Advanced Targeting Conditions: 1.4.6

WordPress version: 6.0.2

PHP version: 7.4.30

Expected behavior

We must not crash the visitor with everbloat risks.

If this level of "everything" tracking is really needed, maybe refactor algo towards LocalStorage?

Current behavior

Visitors hit 400 Bad Request errors because request header size eventually hits server limits.

Steps to reproduce

  1. Verify server configuration to have a limit like https://httpd.apache.org/docs/2.4/mod/core.html#limitrequestfieldsize
  2. Surf site unti cookie grows to exhaust limit

Errors

Server "400 Bad Request" error

Additional context

This directive gives the server administrator greater control over abnormal client request behavior, which may be useful for avoiding some forms of denial-of-service attacks.

So we should not be arbitrarily increasing this to accommodate pum_alm_pages_viewed.

@danieliser
Copy link
Member

@lkraav - Good catch, this one has been out there for a while, never seen that cookie more than a few kb.

I wonder if localStorage might be a better solution?

@lkraav
Copy link
Author

lkraav commented Sep 26, 2022

I wonder if localStorage might be a better solution?

Maybe? Some questions that come to mind:

  • I think localStorage is not directly accessible to PHP, but do you need that anyway for this mechanism's purpose - is it JS-only?
  • Is it OK to grow pum_alm_pages_viewed indefinitely in localStorage?
  • Can pum_alm_pages_viewed be trimmed regardless of storage mechanism (let's say... last 10.. 50.. xyz pages viewed, user configurable) - or does that make its operation inherenly inaccurate?

@saas786
Copy link

saas786 commented Sep 26, 2022

Can pum_alm_pages_viewed be trimmed regardless of storage mechanism (let's say... last 10.. 50.. xyz pages viewed, user configurable) - or does that make its operation inherenly inaccurate?

User configurability can be an additional bonus.

@danieliser
Copy link
Member

@lkraav This condition is not read by the server, its only ever used in JavaScript. Also for cookies, the maximum size is 4096 bytes, whereas for local storage it's 5MB.

For that reason I think we could feasibly go without limits if it moved to localStorage. Will test it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants