@diosmosis opened this Pull Request on August 16th 2020 Member

RequestCommand uses getArrayFromQueryString which doesn't urldecode. (Note: I'm using in this for UI tests in ProxySite since travis-ci will hang (probably deadlock) when using curl requests)

@tsteur commented on August 17th 2020 Member

@diosmosis be good to add a test eg with segments that contains characters like ' " & < > to check it doesn't break anything there? The segment definition would be set when archiving.

@diosmosis commented on August 17th 2020 Member

@tsteur ArchiveCronTest should archive segments (segments defined in ManySitesImportedLogs::getDefaultSegments()). is that enough?

@tsteur commented on August 17th 2020 Member

@diosmosis I'm not sure. they don't seem to be encoded the way they are usually? Eg the 2 archives visitCount<=5;visitorType!=re%2C%3Btest%20is%20encoded;daysSinceFirstVisit<=50 and visitCount<=5;visitorType!=non-existing-type;daysSinceFirstVisit<=50 seem to have actually not valid segment values? I mean the visitorType wouldn't match those values I think. It be great if there was a segment definition like pageUrl=@ and then one or multiple URL encoded characters in the comparison string and this should actually match a value. Then we would know for sure that values are correctly forwarded and read by climulti.

@diosmosis commented on August 18th 2020 Member

@tsteur uses a new segment type, hopefully the next build will pass

@tsteur commented on August 18th 2020 Member

@diosmosis I suppose the changed values in tests are because of the segment change?

@diosmosis commented on August 18th 2020 Member

@tsteur yes, matching less visits now

@diosmosis commented on August 18th 2020 Member

also,when testing I added the changes to 4.x-dev, then switched over and checked the test results were the same on this branch

@tsteur commented on August 18th 2020 Member

👍 LGTM

after some debugging noticed this works eg because we urlencode the segment here: https://github.com/matomo-org/matomo/blob/3.14.1-b1/core/CronArchive.php#L841

This Pull Request was closed on August 18th 2020
Powered by GitHub Issue Mirror