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

Possible issue with unserializing Cookie data #15

Open
trepmal opened this issue Jun 25, 2021 · 0 comments
Open

Possible issue with unserializing Cookie data #15

trepmal opened this issue Jun 25, 2021 · 0 comments

Comments

@trepmal
Copy link
Contributor

trepmal commented Jun 25, 2021

I discovered this while working on #13. I believe I confirmed it's not a regression, but would welcome confirmation.

Issue

When re-flagging a comment, I do not get the expected "already flagged" message.

Reproduce

  1. Activate the plugin on a site running any default theme but Twenty Twenty (See if there are better options than comment_reply_link #14).
  2. Add an approve at least one comment for testing on.
  3. View the comment on the front-end and click "Report Comment"
  4. Note you'll get a sfrc_flags cookie value something like this eyIxOTAzMCI6MX0%3D

The problem is during unserialization

private function unserialize_cookie( $value ) {
$data = json_decode( base64_decode( $value ) );
return $this->clean_cookie_data( $data );
}

Problem one is the base64 decoding leaves a trailing character

wp> $base64 = base64_decode( 'eyIxOTAzMCI6MX0%3D' )
=> string(12) "{"19030":1}7"

Which then prevents json decoding

wp> json_decode( $base64 );
=> NULL

Because the data ends up as null (which is "cleaned" into an empty array), our comment isn't found in the data so is not considered "already flagged".

Note: It may be difficult to fully see this on the front-end due to the transients fallback. Conditional blocks at

if ( $transient = get_transient( md5( $this->_storagecookie . $remote_addr ) ) ) {
and
if ( !$transient ) {
can be commented out for testing.

@trepmal trepmal mentioned this issue Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant