-
Notifications
You must be signed in to change notification settings - Fork 140
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
Strings containing '/n' terminate early when you 'get' them from redis. #24
Comments
Hey Nico, thanks for your extensive bug report. Could you please specify which version of the library you are using? We've been busy restructuring parts of the library (see the develop branch), but I'llhave to test whether this still applies. The Redis protocol uses You can also use double quotes to group these HTML strings, but that would also mean that the double quotes you use in your HTML need to be escaped.. The big question here is whether you should be able to put these kind of things in Redis. Personally I'd say no, but there's no reason to force my opinion on your work. Let me sleep on this for a few nights :) |
Technically we should not care about multiple newlines in a single line reply. A single line reply means read until there's no more to read, not really read to the first newline. I agree that cached HTML is best served (at least on Linux) from the file system as buffers are far more efficient for this - a direct read of memory without the overhead of a socket and protocol. But that's incidental. The problem with this on our end is we're using streams, and A hack to work around this in the library is to use I'm thinking that it's probably a good idea to convert to sockets, but I have no idea on how many hosts don't support them out of paranoia. That's why I didn't just do it when I was dealing with truncated single line replies in excess of 8k length. |
Thanks for the quick replies guys. I am used the latest version (downloaded it from this github page). Ont that note, maybe including the version number in header of the mains files would be good. I used a diff to check that my version was the same as yours :| I am not storing whole html pages in redis just snippets (albeit large ones) that are generated by processing an api. I am using redis rather than a local cache mechanisms so that multiple servers can access the same cache which in turn reduces requests to the api. I cant really comment on any of the socket, fgets stuff but if someone is using redis they are likely already in an situation where they control the environment? |
Thanks for your input Tim. Good point Nico, I tend to pull in the entire Git repo, so I never ran into that problem. Assuming that people can control their environment because they are using Redis sounds like a good premise to me. (Of course there might be some edge cases, like PaaS platforms.) I'll probably do a new release of the latest work in the develop branch somewhere today or tomorrow and start working on native socket support. |
Just a FYI. I released v0.4 with all these changes because the develop branch already included quite a few rigorous changes. I suggest v0.5 will focus on the the native socket rewrite. |
Yeah, definitely the way to go. Sorry for being sporadic, I'm in the midst of changing jobs and houses. The only people that might be put off by this are those using shared hosting and an external Redis host AND Codeigniter, we could probably count the number of people that describes on one hand. I'll get caught up on the dev branch and jump in. |
I was submitting html(generated with php) into redis for caching. When I tried to retrieve the page it would only return part of the string.
After trying to find a bug in my own code. I checked redis which was definitely storing the whole string. Codeignitor redis would only return part of the string. I tested it in Predis which returned the whole string (as expected).
Example:
After playing around with it I realised that string always terminated where an /n appeared in $string (doing a get with redis-cli shows /n in a strings stored).
I tried to find the problem in the codeignitor-redis code but could not.
Work Around Solution:
Do a string replace before setting a string that could contain \n. Hopefully this helps someone else who experienced the same thing.
Sorry if this is not a good bug report. It is my first attempt at one.
The text was updated successfully, but these errors were encountered: