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 in file cache backend #5083

Open
Arantor opened this Issue Oct 23, 2018 · 8 comments

Comments

Projects
None yet
4 participants
@Arantor
Contributor

Arantor commented Oct 23, 2018

Description

There's always been some concern about the reliability of the file cache backend. I think we have a case where it actively has been shown to break.

If the cached content contains a null byte, the addcslashes call to write the data will truncate the string.

Steps to reproduce

  1. Have content that contains a null byte
  2. Push it to the cache with cache_put_data
  3. Observe result file cache is malformed

Environment (complete as necessary)

  • Version/Git revision: master
  • Database Type: n/a
  • Database Version: n/a
  • PHP Version: n/a

Additional information/references

See https://www.simplemachines.org/community/index.php?topic=560471.0 for more including a dump of a modSettings cache entry at 20489 bytes, so not a file write boundary issue (like we saw back in the day). I don't know for sure if this is the problem but it's a reasonable bet right now.

@Gwenwyfar Gwenwyfar added the Caching label Oct 23, 2018

@Sesquipedalian

This comment has been minimized.

Member

Sesquipedalian commented Oct 23, 2018

Interesting. I find myself wondering if there are any cases where we would ever want to write a null byte into the cache. If not, perhaps an easy solution would be a simple str_replace("\0", '', $str) when reading and/or writing cache files.

@Sesquipedalian

This comment has been minimized.

Member

Sesquipedalian commented Oct 23, 2018

...Or maybe just convert any null characters to� entities on write, and convert them back on read.

@jdarwood007

This comment has been minimized.

Member

jdarwood007 commented Oct 23, 2018

After we convert it to a string, we could convert it to a hex.
bin2hex and hex2bin could possibly do this.

Adds some overhead though.

@Sesquipedalian

This comment has been minimized.

Member

Sesquipedalian commented Oct 23, 2018

Hex wouldn't be an efficient solution. https://stackoverflow.com/a/2612978

@jdarwood007

This comment has been minimized.

Member

jdarwood007 commented Oct 24, 2018

Well your right. We could try to add into it to use pack/unpack with compressions, but I think at that point we are wasting more cycles than we are saving.

@Sesquipedalian

This comment has been minimized.

Member

Sesquipedalian commented Oct 24, 2018

str_replace("\0", '�', $str) (and vice versa on read) is about as efficient as we could get, I think.

@Arantor

This comment has been minimized.

Contributor

Arantor commented Oct 24, 2018

Or even use the correct escaping as I suggested in topic 😉

@Sesquipedalian

This comment has been minimized.

Member

Sesquipedalian commented Oct 25, 2018

Looks at topic again. Notices post on second page

Oh, yes. That would work better. 👍

@Sesquipedalian Sesquipedalian added this to the RC 2 milestone Oct 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment