diff --git a/integration_tests/tests/web/test_get_url_parameter_substring.sql b/integration_tests/tests/web/test_get_url_parameter_substring.sql new file mode 100644 index 000000000..f12d3fdb4 --- /dev/null +++ b/integration_tests/tests/web/test_get_url_parameter_substring.sql @@ -0,0 +1,115 @@ +-- Regression tests for https://github.com/dbt-labs/dbt-utils/issues/980 +-- +-- `get_url_parameter` must only match a parameter when the search token +-- appears as a complete parameter name — not when the token happens to be +-- a suffix of a different parameter name. The cases below all used to +-- return the matched substring's value rather than NULL. + +with cases as ( + + select + 'http://example.com/?msg=hello' as url, + 'g' as param, + cast(null as {{ dbt.type_string() }}) as expected + + union all + + select + 'http://example.com/?sku=EXAMPLE_SKU&u=https%3A%2F%2Fx.com', + 'ku', + cast(null as {{ dbt.type_string() }}) + + union all + + -- Legitimate short-name parameter matches must still resolve correctly. + select + 'http://example.com/?m=first_value&s=second_value', + 'm', + 'first_value' + + union all + + select + 'http://example.com/?m=first_value&s=second_value', + 's', + 'second_value' + + union all + + -- Existing behaviour must be preserved. + select + 'http://example.com/?utm_medium=organic&utm_source=github', + 'utm_source', + 'github' + + union all + + select + 'http://example.com/?utm_medium=organic&utm_source=github', + 'utm_medium', + 'organic' + +), + +comparisons as ( + + select + {{ dbt_utils.get_url_parameter('url', 'g') }} as actual, + case when param = 'g' then expected end as expected, + param + from cases + where param = 'g' + + union all + + select + {{ dbt_utils.get_url_parameter('url', 'ku') }} as actual, + case when param = 'ku' then expected end as expected, + param + from cases + where param = 'ku' + + union all + + select + {{ dbt_utils.get_url_parameter('url', 'm') }} as actual, + case when param = 'm' then expected end as expected, + param + from cases + where param = 'm' + + union all + + select + {{ dbt_utils.get_url_parameter('url', 's') }} as actual, + case when param = 's' then expected end as expected, + param + from cases + where param = 's' + + union all + + select + {{ dbt_utils.get_url_parameter('url', 'utm_source') }} as actual, + case when param = 'utm_source' then expected end as expected, + param + from cases + where param = 'utm_source' + + union all + + select + {{ dbt_utils.get_url_parameter('url', 'utm_medium') }} as actual, + case when param = 'utm_medium' then expected end as expected, + param + from cases + where param = 'utm_medium' + +) + +-- A row is a failure if actual and expected disagree, treating NULLs as equal. +select * +from comparisons +where + (actual is null) <> (expected is null) + or coalesce(actual, '') <> coalesce(expected, '') diff --git a/macros/web/get_url_parameter.sql b/macros/web/get_url_parameter.sql index 8147b41f5..a12dc3eac 100644 --- a/macros/web/get_url_parameter.sql +++ b/macros/web/get_url_parameter.sql @@ -4,9 +4,19 @@ {% macro default__get_url_parameter(field, url_parameter) -%} -{%- set formatted_url_parameter = "'" + url_parameter + "='" -%} +{#- + Normalize the query string so every parameter is preceded by `&`, then split on + `&=`. Requiring the leading `&` in the search token prevents false + matches against parameters whose names merely *end* with `url_parameter` + (e.g. searching for `g` inside `...&msg=...`). + See: https://github.com/dbt-labs/dbt-utils/issues/980 +-#} -{%- set split = dbt.split_part(dbt.split_part(field, formatted_url_parameter, 2), "'&'", 1) -%} +{%- set formatted_url_parameter = "'&" + url_parameter + "='" -%} + +{%- set normalized_field = dbt.concat(["'&'", dbt.replace(field, "'?'", "'&'")]) -%} + +{%- set split = dbt.split_part(dbt.split_part(normalized_field, formatted_url_parameter, 2), "'&'", 1) -%} nullif({{ split }},'')