Re: [PATCH] RFC: Allow modification/deletion of specific headers in apr_table_t
Sumeet Singh wrote: > Brian, >If you ask me, I can write the new functions (i.e. the apr_table_do - > type functions you described) and submit a patch for review. Let me know I have often wanted an apr_table_do-esque interface that allowed for modification of table entries as suggested, so that sounds good to me. --Geoff
Re: [PATCH] RFC: Allow modification/deletion of specific headers in apr_table_t
Brian, If you ask me, I can write the new functions (i.e. the apr_table_do - type functions you described) and submit a patch for review. Let me know ... -sumeet Sumeet Singh wrote: Brian Pane wrote: On Apr 21, 2004, at 9:11 AM, Sumeet Singh wrote: Hi, In my use-case I am dealing with multiple headers with the same key (e.g. Cookie), and need to modify or remove a specific header based on its key and value (e.g. remove a certain cookie header while leaving the rest in). There is no api in apr_tables that would allow me to remove a given header instance. apr_table_unset will remove all cookies. And I can't have my code remove a header from the apr_array_table_t array because that will render the internal hash index incorrect. Secondly, eventhough I can modify the value of a specific header by iterating over the apr_array_header_t, that would be inefficient because I wouldn't be able to use the internal index_first and index_last values. Therefore I have written three functions (patch files attached) and am hoping that the powers-that-be will be willing to review and roll them into the APR codeline. 1) apr_table_set_one (apr_table_t *t, const char *key, const char *oldVal, const char *newVal) replaces value of header "key" and value "oldVal" with value "newVal". If "oldVal" is null, then the first occurance of the header is replaces (this is an optimization for situations where we know that only one header exists and don't care about its current value). If the header is not found, then it behaves like apr_table_add. I finally got a chance to read through the patch. The code makes sense, but I have a question: will apr_table_set_one and apr_table_unset_one provide all the functionality you need? Using Cookie and Set-Cookie headers as an example, it seems like an exact match on the oldVal wouldn't be sufficient. If your response headers include something like this: Content-Type; text/html Set-Cookie: userID=1; max-age=86400; domain=.example.com Cache-Control: private Set-Cookie: sessionID=54321; max-age=1800; domain=.example.com and you want to change or remove the "userID" cookie, it's a lot easier to do a prefix match on "userID=" than on the entire value of that table entry. (If that table entry was set by some other module, we may not know what the full value of that entry is.) That's a good point. In the general case, an application might need to: 1. iterate through all the elements in a table that match a given key, 2. selectively modify the value of one or more of the matching elements, or delete one or more of the matching elements, 3. and optionally stop iterating based on some application-defined criteria. Agreed. This sounds a lot like apr_table_do, except that apr_table_do doesn't allow modification of the table elements. If there were a variant of apr_table_do that allowed modification of the value (but not the key) of visited elements, and the deletion of visited elements, would that work for your application? Yes that should suffice. Thanks. -sumeet Brian
Re: [PATCH] RFC: Allow modification/deletion of specific headers in apr_table_t
Brian Pane wrote: On Apr 21, 2004, at 9:11 AM, Sumeet Singh wrote: Hi, In my use-case I am dealing with multiple headers with the same key (e.g. Cookie), and need to modify or remove a specific header based on its key and value (e.g. remove a certain cookie header while leaving the rest in). There is no api in apr_tables that would allow me to remove a given header instance. apr_table_unset will remove all cookies. And I can't have my code remove a header from the apr_array_table_t array because that will render the internal hash index incorrect. Secondly, eventhough I can modify the value of a specific header by iterating over the apr_array_header_t, that would be inefficient because I wouldn't be able to use the internal index_first and index_last values. Therefore I have written three functions (patch files attached) and am hoping that the powers-that-be will be willing to review and roll them into the APR codeline. 1) apr_table_set_one (apr_table_t *t, const char *key, const char *oldVal, const char *newVal) replaces value of header "key" and value "oldVal" with value "newVal". If "oldVal" is null, then the first occurance of the header is replaces (this is an optimization for situations where we know that only one header exists and don't care about its current value). If the header is not found, then it behaves like apr_table_add. I finally got a chance to read through the patch. The code makes sense, but I have a question: will apr_table_set_one and apr_table_unset_one provide all the functionality you need? Using Cookie and Set-Cookie headers as an example, it seems like an exact match on the oldVal wouldn't be sufficient. If your response headers include something like this: Content-Type; text/html Set-Cookie: userID=1; max-age=86400; domain=.example.com Cache-Control: private Set-Cookie: sessionID=54321; max-age=1800; domain=.example.com and you want to change or remove the "userID" cookie, it's a lot easier to do a prefix match on "userID=" than on the entire value of that table entry. (If that table entry was set by some other module, we may not know what the full value of that entry is.) That's a good point. In the general case, an application might need to: 1. iterate through all the elements in a table that match a given key, 2. selectively modify the value of one or more of the matching elements, or delete one or more of the matching elements, 3. and optionally stop iterating based on some application-defined criteria. Agreed. This sounds a lot like apr_table_do, except that apr_table_do doesn't allow modification of the table elements. If there were a variant of apr_table_do that allowed modification of the value (but not the key) of visited elements, and the deletion of visited elements, would that work for your application? Yes that should suffice. Thanks. -sumeet Brian
Re: [PATCH] RFC: Allow modification/deletion of specific headers in apr_table_t
On Apr 21, 2004, at 9:11 AM, Sumeet Singh wrote: Hi, In my use-case I am dealing with multiple headers with the same key (e.g. Cookie), and need to modify or remove a specific header based on its key and value (e.g. remove a certain cookie header while leaving the rest in). There is no api in apr_tables that would allow me to remove a given header instance. apr_table_unset will remove all cookies. And I can't have my code remove a header from the apr_array_table_t array because that will render the internal hash index incorrect. Secondly, eventhough I can modify the value of a specific header by iterating over the apr_array_header_t, that would be inefficient because I wouldn't be able to use the internal index_first and index_last values. Therefore I have written three functions (patch files attached) and am hoping that the powers-that-be will be willing to review and roll them into the APR codeline. 1) apr_table_set_one (apr_table_t *t, const char *key, const char *oldVal, const char *newVal) replaces value of header "key" and value "oldVal" with value "newVal". If "oldVal" is null, then the first occurance of the header is replaces (this is an optimization for situations where we know that only one header exists and don't care about its current value). If the header is not found, then it behaves like apr_table_add. I finally got a chance to read through the patch. The code makes sense, but I have a question: will apr_table_set_one and apr_table_unset_one provide all the functionality you need? Using Cookie and Set-Cookie headers as an example, it seems like an exact match on the oldVal wouldn't be sufficient. If your response headers include something like this: Content-Type; text/html Set-Cookie: userID=1; max-age=86400; domain=.example.com Cache-Control: private Set-Cookie: sessionID=54321; max-age=1800; domain=.example.com and you want to change or remove the "userID" cookie, it's a lot easier to do a prefix match on "userID=" than on the entire value of that table entry. (If that table entry was set by some other module, we may not know what the full value of that entry is.) In the general case, an application might need to: 1. iterate through all the elements in a table that match a given key, 2. selectively modify the value of one or more of the matching elements, or delete one or more of the matching elements, 3. and optionally stop iterating based on some application-defined criteria. This sounds a lot like apr_table_do, except that apr_table_do doesn't allow modification of the table elements. If there were a variant of apr_table_do that allowed modification of the value (but not the key) of visited elements, and the deletion of visited elements, would that work for your application? Brian
Re: [PATCH] RFC: Allow modification/deletion of specific headers in apr_table_t
On Apr 21, 2004, at 9:11 AM, Sumeet Singh wrote: Hi, In my use-case I am dealing with multiple headers with the same key (e.g. Cookie), and need to modify or remove a specific header based on its key and value (e.g. remove a certain cookie header while leaving the rest in). There is no api in apr_tables that would allow me to remove a given header instance. apr_table_unset will remove all cookies. And I can't have my code remove a header from the apr_array_table_t array because that will render the internal hash index incorrect. Secondly, eventhough I can modify the value of a specific header by iterating over the apr_array_header_t, that would be inefficient because I wouldn't be able to use the internal index_first and index_last values. Therefore I have written three functions (patch files attached) and am hoping that the powers-that-be will be willing to review and roll them into the APR codeline. 1) apr_table_set_one (apr_table_t *t, const char *key, const char *oldVal, const char *newVal) replaces value of header "key" and value "oldVal" with value "newVal". If "oldVal" is null, then the first occurance of the header is replaces (this is an optimization for situations where we know that only one header exists and don't care about its current value). If the header is not found, then it behaves like apr_table_add. 2) apr_table_setn_one(apr_table_t *t, const char *key, const char *oldVal, const char *newVal) Same as apr_table_set_one exept that it doesn't make a copy of key and newVal. 3) apr_table_unset_one(apr_table_t *t, const char *key, const char *oldVal, const char *newVal) Unsets header "key" with value "oldVal". If "oldVal" is null, then the first instance of the header (only) is unset (this is an optimization for situations where we know that only one header exists and don't care about its current value). +1 for the concept. I'm willing to review and commit the code if nobody else has any objections in the next couple of days. -Brian
[PATCH] RFC: Allow modification/deletion of specific headers in apr_table_t
Hi,
In my use-case I am dealing with multiple headers with the same key
(e.g. Cookie), and need to modify or remove a specific header based on
its key and value (e.g. remove a certain cookie header while leaving the
rest in). There is no api in apr_tables that would allow me to remove a
given header instance. apr_table_unset will remove all cookies. And I
can't have my code remove a header from the apr_array_table_t array
because that will render the internal hash index incorrect. Secondly,
eventhough I can modify the value of a specific header by iterating over
the apr_array_header_t, that would be inefficient because I wouldn't be
able to use the internal index_first and index_last values. Therefore I
have written three functions (patch files attached) and am hoping that
the powers-that-be
will be willing to review and roll them into the APR codeline.
1) apr_table_set_one (apr_table_t *t, const char *key, const char
*oldVal, const char *newVal)
replaces value of header "key" and value "oldVal" with value
"newVal". If "oldVal" is null, then the first occurance of the header is
replaces (this is an optimization for situations where we know that only
one header exists and don't care about its current value). If the
header is not found, then it behaves like apr_table_add.
2) apr_table_setn_one(apr_table_t *t, const char *key, const char
*oldVal, const char *newVal)
Same as apr_table_set_one exept that it doesn't make a copy of key
and newVal.
3) apr_table_unset_one(apr_table_t *t, const char *key, const char
*oldVal, const char *newVal)
Unsets header "key" with value "oldVal". If "oldVal" is null, then
the first instance of the header (only) is unset (this is an
optimization for situations where we know that only one header exists
and don't care about its current value).
-regards,
sumeet
*** apr_tables.cFri Feb 13 01:38:34 2004
--- apr_tables++.c Thu Apr 15 18:39:26 2004
***
*** 1204,1206
--- 1204,1304
apr_table_compress(a, flags);
}
+
+ static void apr_table_set_one_internal(apr_table_t *t,
+const char *key,
+const char *oldVal,
+const char *newVal,
+int copy)
+ {
+ apr_table_entry_t *next_elt;
+ apr_table_entry_t *end_elt;
+ apr_table_entry_t *table_end;
+ apr_uint32_t checksum;
+ int hash;
+
+ COMPUTE_KEY_CHECKSUM(key, checksum);
+ hash = TABLE_HASH(key);
+ if (!TABLE_INDEX_IS_INITIALIZED(t, hash)) {
+ t->index_first[hash] = t->a.nelts;
+ TABLE_SET_INDEX_INITIALIZED(t, hash);
+ goto add_new_elt;
+ }
+ next_elt = ((apr_table_entry_t *) t->a.elts) + t->index_first[hash];;
+ end_elt = ((apr_table_entry_t *) t->a.elts) + t->index_last[hash];
+ table_end =((apr_table_entry_t *) t->a.elts) + t->a.nelts;
+
+ for (; next_elt <= end_elt; next_elt++) {
+ if ((checksum == next_elt->key_checksum) &&
+ !strcasecmp(next_elt->key, key) &&
+ (!oldVal || !strcmp(next_elt->val, oldVal))) {
+
+ /* Found An Existing Entry With The Same Key, So Overwrite It */
+ next_elt->val = copy ? apr_pstrdup(t->a.pool, newVal)
+ : (char *) newVal;
+ return;
+ }
+ }
+
+ add_new_elt:
+ t->index_last[hash] = t->a.nelts;
+ next_elt = (apr_table_entry_t *) table_push(t);
+ next_elt->key = copy ? apr_pstrdup(t->a.pool, key): (char *) key;
+ next_elt->val = copy ? apr_pstrdup(t->a.pool, newVal): (char *) newVal;
+ next_elt->key_checksum = checksum;
+ }
+
+ APR_DECLARE(void) apr_table_set_one(apr_table_t *t, const char *key, const char
*oldVal,
+const char *newVal)
+ {
+ apr_table_set_one_internal(t, key, oldVal, newVal, 1);
+ return;
+ }
+
+ APR_DECLARE(void) apr_table_setn_one(apr_table_t* t, const char *key, const char
*oldVal,
+ const char *newVal)
+ {
+ apr_table_set_one_internal(t, key, oldVal, newVal, 0);
+ return;
+ }
+
+ APR_DECLARE(void) apr_table_unset_one(apr_table_t *t, const char *key, const char*
val)
+ {
+ apr_table_entry_t *next_elt;
+ apr_table_entry_t *end_elt;
+ apr_table_entry_t *dst_elt;
+ apr_uint32_t checksum;
+ int hash;
+ int must_reindex;
+
+ hash = TABLE_HASH(key);
+ if (!TABLE_INDEX_IS_INITIALIZED(t, hash)) {
+ return;
+ }
+ COMPUTE_KEY_CHECKSUM(key, checksum);
+ next_elt = ((apr_table_entry_t *) t->a.elts) + t->index_first[hash];
+ end_elt = ((apr_table_entry_t *) t->a.elts) + t->index_last[hash];
+ must_reindex = 0;
+ for (; next_elt <= end_elt; next_elt++) {
+ if ((checksum == next_elt->key_checksum) &&
+ !strcasecmp(next_elt->key, key) &&
+ (!val || !strcmp(next_elt->val, val))) {
+ /* Fou
