Fix issue #1780: correct keyword expansion when URL or Author value contains '$'. Patch by: Lieven Govaerts * subversion/libsvn_subr/subst.c (translate_chunk): Keep searching for next '$' until valid keyword is found or search limits are reached. (translate_chunk): add algorithm that keeps searching for next '$' in the value of a valid expanded keyword until a correct value is found or search limits are reached. (match_keyword): new local function that matches a valid keyword in a char buffer. (translate_keyword): added keyword_name parameter, translate a keyword keyword_name with its value. Keyword matching was put in match_keyword. * subversion/tests/libsvn_wc/translate-test.c (lines): Add lines to test on '$' in Author and URL keywords. (substitute_and_verify): Index: subversion/libsvn_subr/subst.c =================================================================== --- subversion/libsvn_subr/subst.c (revision 18450) +++ subversion/libsvn_subr/subst.c (working copy) @@ -607,11 +607,34 @@ } /* Parse BUF (whose length is *LEN) for Subversion keywords. If a - keyword is found, optionally perform the substitution on it in - place, update *LEN with the new length of the translated keyword - string, and return TRUE. If this buffer doesn't contain a known - keyword pattern, leave BUF and *LEN untouched and return FALSE. + keyword is found, update *KEYWORD_NAME with the keyword name and + return TRUE. */ +static svn_boolean_t +match_keyword (char *buf, + apr_size_t *len, + char *keyword_name, + apr_hash_t *keywords) +{ + apr_size_t i; + /* Early return for ignored keywords */ + if (! keywords) + return FALSE; + + /* Extract the name of the keyword */ + for (i = 0; i < *len - 2 && buf[i + 1] != ':'; i++) + keyword_name[i] = buf[i + 1]; + keyword_name[i] = '\0'; + + return apr_hash_get (keywords, keyword_name, APR_HASH_KEY_STRING) != NULL; +} + +/* Try to translate keyword *KEYWORD_NAME in BUF: + optionally perform the substitution in place, update *LEN with + the new length of the translated keyword string, and return TRUE. + If this buffer doesn't contain a known keyword pattern, leave BUF + and *LEN untouched and return FALSE. + See the docstring for svn_subst_copy_and_translate for how the EXPAND and KEYWORDS parameters work. @@ -625,12 +648,11 @@ static svn_boolean_t translate_keyword (char *buf, apr_size_t *len, + const char *keyword_name, svn_boolean_t expand, apr_hash_t *keywords) { const svn_string_t *value; - char keyword_name[SVN_KEYWORD_MAX_LEN + 1]; - apr_size_t i; /* Make sure we gotz good stuffs. */ assert (*len <= SVN_KEYWORD_MAX_LEN); @@ -640,11 +662,6 @@ if (! keywords) return FALSE; - /* Extract the name of the keyword */ - for (i = 0; i < *len - 2 && buf[i + 1] != ':'; i++) - keyword_name[i] = buf[i + 1]; - keyword_name[i] = '\0'; - value = apr_hash_get (keywords, keyword_name, APR_HASH_KEY_STRING); if (value) @@ -919,6 +936,7 @@ /* precalculate some oft-used values */ const char *end = buf + buflen; const char *interesting = b->interesting; + apr_size_t next_sign_off = 0; /* At the beginning of this loop, assume that we might be in an * interesting state, i.e. with data in the newline or keyword @@ -943,21 +961,50 @@ } else if (b->keyword_off && *p == '$') { - /* If translation fails, treat this '$' as a starting '$'. */ - b->keyword_buf[b->keyword_off++] = '$'; - if (translate_keyword (b->keyword_buf, &b->keyword_off, - b->expand, b->keywords)) - p++; + svn_boolean_t keyword_matches; + char keyword_name[SVN_KEYWORD_MAX_LEN + 1]; + + /* If keyword is matched, but not correctly translated, try to + * look for the next ending '$'. */ + b->keyword_buf[b->keyword_off++] = *p++; + keyword_matches = match_keyword (b->keyword_buf, &b->keyword_off, + keyword_name, b->keywords); + if (keyword_matches == FALSE) + { + /* reuse the ending '$' */ + *p--; + b->keyword_off--; + } + + if (keyword_matches == FALSE || + translate_keyword (b->keyword_buf, &b->keyword_off, + keyword_name, b->expand, b->keywords) || + b->keyword_off >= SVN_KEYWORD_MAX_LEN) + { + /* write out non-matching text or translated keyword */ + SVN_ERR (translate_write (dst, b->keyword_buf, b->keyword_off)); + + next_sign_off = 0; + b->keyword_off = 0; + } else - b->keyword_off--; + { + if (next_sign_off == 0) + next_sign_off = b->keyword_off - 1; - SVN_ERR (translate_write (dst, b->keyword_buf, b->keyword_off)); - - b->keyword_off = 0; + continue; + } } else if (b->keyword_off == SVN_KEYWORD_MAX_LEN - 1 || (b->keyword_off && (*p == '\r' || *p == '\n'))) { + if (next_sign_off > 0) + { + /* rolling back, continue with next '$' in keyword_buf */ + p -= (b->keyword_off - next_sign_off); + b->keyword_off = next_sign_off; + next_sign_off = 0; + } /* No closing '$' found; flush the keyword buffer. */ SVN_ERR (translate_write (dst, b->keyword_buf, b->keyword_off)); Index: subversion/tests/libsvn_wc/translate-test.c =================================================================== --- subversion/tests/libsvn_wc/translate-test.c (revision 18450) +++ subversion/tests/libsvn_wc/translate-test.c (working copy) @@ -126,7 +126,45 @@ ".$veR$Author$", "$", "$$", - "Line 74: end of subst test data." + /* Line 74-75 test for keywords containing '$', issue #1780 */ + "Line 74: Valid $Author: jran$dom $, started expanded.", + "Line 75: Valid $URL: http://tomato/mau$ve $, started expanded.", + /* Line 76-78 tests for a string with an unknown keyword of 252-254 bytes + long */ + "$ " + " " + " " + " $$", + "$ " + " " + " " + " $$", + "$ " + " " + " " + " $$", + /* Testing border cases, line 79&80 test for valid keywords, keywords on + line 81&82 are too long */ + "Line 79: Valid $Author: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaa$aaaaaaaaaaaaaaaaaaaaaa$aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa $, started expanded.", + "Line 80: Valid $Author: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$aaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$aaaaaaaaaaaaaaaaaa" + "aaaaaaa$aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$aaaaaaa $, started " + "expanded.", + "Line 81: Invalid $Author: aaaaaaaaaaaaaaaaaaaa$aaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaa$aaaaaaaaaaaaaaaaaaaa$$aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$$aaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$$aaaaaaaaaaaaa $, started " + "expanded.", + "Line 82: Invalid $Author: aaaaaaaaaaa$$aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaa$$$$aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa$$aaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaa$$aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa $, started " + "expanded.", + "Line 83: end of subst test data." }; @@ -468,6 +506,19 @@ NULL); expect[71 - 1] = apr_pstrcat (pool, ".$veR$Author: ", author, " $", NULL); + + expect[74 - 1] = + apr_pstrcat (pool, "Line 74: ", + "Valid $Author: ", author, " $, started expanded.", + NULL); + expect[79 - 1] = + apr_pstrcat (pool, "Line 79: ", + "Valid $Author: ", author, " $, started expanded.", + NULL); + expect[80 - 1] = + apr_pstrcat (pool, "Line 80: ", + "Valid $Author: ", author, " $, started expanded.", + NULL); } else /* unexpand */ { @@ -476,6 +527,12 @@ "Line 37: Valid $LastChangedBy$, started expanded."; expect[38 - 1] = "Line 38: Valid $Author$, started expanded."; + expect[74 - 1] = + "Line 74: Valid $Author$, started expanded."; + expect[79 - 1] = + "Line 79: Valid $Author$, started expanded."; + expect[80 - 1] = + "Line 80: Valid $Author$, started expanded."; } } @@ -499,6 +556,10 @@ apr_pstrcat (pool, "Line 42: ", "Valid $URL: ", url, " $, started expanded.", NULL); + expect[75 - 1] = + apr_pstrcat (pool, "Line 75: ", + "Valid $URL: ", url, " $, started expanded.", + NULL); } else /* unexpand */ { @@ -507,6 +568,8 @@ "Line 41: Valid $HeadURL$, started expanded."; expect[42 - 1] = "Line 42: Valid $URL$, started expanded."; + expect[75 - 1] = + "Line 75: Valid $URL$, started expanded."; } }