+1 on config cleanup On Sep 11, 2013 8:01 PM, "James Peach" <jpe...@apache.org> wrote:
> On Sep 10, 2013, at 10:18 PM, Yongming Zhao <ming....@gmail.com> wrote: > > > > > we have a way to insert and remove the Client-ip header, which is to > record the UA ip address: > > > > proxy.config.http.anonymize_remove_client_ip > > > > INT > > > > 0 > > > > When enabled (1), Traffic Server removes Client-IP headers for more > privacy. > > > > proxy.config.http.anonymize_insert_client_ip > > > > INT > > > > 1 > > > > When enabled (1), Traffic Server inserts Client-IP headers to retain the > client IP address. > > > > > > that is really strange in directive name, as > > > > we have anonymize for remove sensitive headers: > > > > zymMBPr:mgmt zym$ grep anonymize RecordsConfig.cc > > {RECT_CONFIG, "proxy.config.http.anonymize_remove_from", RECD_INT, "0", > RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} > > {RECT_CONFIG, "proxy.config.http.anonymize_remove_referer", RECD_INT, > "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} > > {RECT_CONFIG, "proxy.config.http.anonymize_remove_user_agent", > RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} > > {RECT_CONFIG, "proxy.config.http.anonymize_remove_cookie", RECD_INT, > "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} > > {RECT_CONFIG, "proxy.config.http.anonymize_remove_client_ip", RECD_INT, > "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} > > {RECT_CONFIG, "proxy.config.http.anonymize_insert_client_ip", RECD_INT, > "1", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} > > {RECT_CONFIG, "proxy.config.http.anonymize_other_header_list", > RECD_STRING, NULL, RECU_DYNAMIC, RR_NULL, RECC_STR, ".*", RECA_NULL} > > > > we do have another way to insert some useful headers: > > > > zymMBPr:mgmt zym$ grep insert RecordsConfig.cc > > {RECT_CONFIG, "proxy.config.http.insert_request_via_str", RECD_INT, > "1", RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL} > > {RECT_CONFIG, "proxy.config.http.insert_response_via_str", RECD_INT, > "0", RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL} > > {RECT_CONFIG, "proxy.config.http.anonymize_insert_client_ip", RECD_INT, > "1", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} > > {RECT_CONFIG, "proxy.config.http.insert_squid_x_forwarded_for", > RECD_INT, "1", RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL} > > {RECT_CONFIG, "proxy.config.http.insert_age_in_response", RECD_INT, > "1", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} > > > > > > but we have now both of the name: > > > > {RECT_CONFIG, "proxy.config.http.anonymize_remove_client_ip", RECD_INT, > "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} > > {RECT_CONFIG, "proxy.config.http.anonymize_insert_client_ip", RECD_INT, > "1", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} > > > > > > we should name proxy.config.http.anonymize_insert_client_ip as > 'proxy.config.http.insert_request_client_ip' or just > 'proxy.config.http.insert_client_ip' > > > > and the current implement of the insert client ip works only if the > client request do not have a 'Client-ip' header, but sometimes we need to > replace it even if someone send us a fake 'Client-ip': > > > > proxy.config.http.insert_client_ip > > > > INT > > > > 1 > > > > > > When disabled(0), do nothing. > > When enabled (1), Traffic Server inserts Client-IP headers to retain the > client IP address, if there is no such headers. > > When forced (2), Traffic Server inserts Client-IP, or replace the origin > Client-IP header if it is already there. > > > > > > any comments? > > I like this. I think that it's worth being consistent and predictable. The > ability to forcibly insert the ClientIP is also useful. I'd like to see > this addition in the 4.x series, with deprecation warnings for the old > names. Once 5.x comes around, we can remove the old, inconsistent names > > J