Ruediger Pluem wrote:
+AP_DECLARE(int) ap_unescape_all(char *url); + +/**Doesn't this require a minor bump?
I think it does do, yes. Fixed.
+AP_DECLARE(char *) ap_escape_path_segment_b(char *c, const char *s);This is not a very descriptive name. Shouldn't it be better ap_escape_path_segment_buffer?
It was originally buffer, then I changed it because it was too long. You're right though, will change.
+AP_DECLARE(void) ap_session_get(request_rec * r, session_rec * z, const char *key, const char **value)+{ + if (!z) { + ap_session_load(r, &z);Not checking the return value can lead to segfaults as z can be invalid.
The real check is to ensure that z is set, the return value can be successful but z can still be NULL (which will happen if no session is
configured), so checking the return value won't help. Fixed.
+ /* decode what we have */ + encoded = apr_pstrcat(r->pool, z->encoded, NULL);What is the purpose of this? Did you mean apr_pstrdup? And why doing a copy at all if you set z->encoded to NULL later anyway?+ pair = apr_strtok(encoded, sep, &last);
It's because of the line above - strtok stomps all over the buffer, the copy ensures the buffer really is ours to stomp on. Sure we set it to NULL later, but there is no guarantee that someone else doesn't hold a pointer to z->encoded somewhere.
+ session_dir_conf *new = + (session_dir_conf *) apr_pcalloc(p, sizeof(session_dir_conf)); + + new->includes = apr_array_make(p, 10, sizeof(const char **)); + new->excludes = apr_array_make(p, 10, sizeof(const char **));Not that it really matters, but aren't the elements of the arrays char * instead of char **?
Not sure, I think they weren't but it was late when I wrote that, let me double check.
Regards, Graham --
smime.p7s
Description: S/MIME Cryptographic Signature
