beetlebugorg / mod_dims

Apache HTTP dynamic image resizing module
Other
43 stars 27 forks source link

mix of position-dependent and -independent logic #7

Closed rflynn closed 11 years ago

rflynn commented 11 years ago

https://github.com/beetlebugorg/mod_dims/blob/master/src/mod_dims.c#L1287

mixes position-independent use of string searching "strstr()" and position-dependent pointer arithmetic to check for urls that start with "http:/" but not "http://"... use strncmp() or memcp() instead.

beetlebugorg commented 11 years ago

This code is checking for the resize URL that starts like http:/foobar.com/image.png. The r->uri value would be something like "/thumbnail/100x100/http:/foobar.com/image.png".

I'm not sure I understand how the use of strstr() is wrong. strstr() returns a pointer to the first occurrence of http:/ or null if it cannot find it. The pointer arithmetic should be safe on the return value.

Any chance you can implement it the way you think is correct and submit a pull request?

beetlebugorg commented 11 years ago

Copying these other locations from issue #8.

https://github.com/beetlebugorg/mod_dims/blob/master/src/mod_dims.c#L1286 https://github.com/beetlebugorg/mod_dims/blob/master/src/mod_dims.c#L1372 https://github.com/beetlebugorg/mod_dims/blob/master/src/mod_dims.c#L1430

rflynn commented 11 years ago

Ah, my mistake. I thought it was checking only the prefix. Thanks for the explanation.