Popup-Maker icon indicating copy to clipboard operation
Popup-Maker copied to clipboard

Advanced Targeting Conditions: `pum_alm_pages_viewed` cookie size everbloat

Open lkraav opened this issue 2 years ago • 4 comments

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.

lkraav avatar Sep 22 '22 11:09 lkraav

@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?

danieliser avatar Sep 24 '22 00:09 danieliser

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?

lkraav avatar Sep 26 '22 11:09 lkraav

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.

saas786 avatar Sep 26 '22 14:09 saas786

@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.

danieliser avatar Jan 20 '23 09:01 danieliser