Fix exception in predefined HTTP handler on absent header by vitlibar · Pull Request #108324 · ClickHouse/ClickHouse · GitHub
Skip to content

Fix exception in predefined HTTP handler on absent header#108324

Merged
vitlibar merged 1 commit into
ClickHouse:masterfrom
vitlibar:fix-http-handler-header-regex-missing-header
Jun 24, 2026
Merged

Fix exception in predefined HTTP handler on absent header#108324
vitlibar merged 1 commit into
ClickHouse:masterfrom
vitlibar:fix-http-handler-header-regex-missing-header

Conversation

@vitlibar

@vitlibar vitlibar commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix exception in predefined HTTP handler on absent header

The problem was found in PR #107492, it's that when a regular expression for a HTTP header allows empty string:

<clickhouse>
    <http_handlers>
        <rule>
            <methods>GET</methods>
            <url>/test_predefined_handler_absent_header</url>
            <headers_regexp><XXX><![CDATA[(?P<value_from_header>.*)]]></XXX></headers_regexp>
            <handler>
                <type>predefined_query_handler</type>
                <query>SELECT {value_from_header:String}</query>
            </handler>
        </rule>
    </http_handlers>
</clickhouse>

then it matches if a request doesn't have this header at all, but before this PR then it threw an exception trying to extract the value of a named capturing group from this header.

@clickhouse-gh

clickhouse-gh Bot commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Jun 23, 2026
@vitlibar vitlibar mentioned this pull request Jun 23, 2026
Merged
vitlibar
vitlibar commented Jun 23, 2026
View reviewed changes
Comment thread tests/integration/test_http_handlers_config/test.py
assert response.content == b"hello\n"

# Header absent: the rule still matches and the captured parameter is empty.
response = get({})

@vitlibar vitlibar Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it threw an exception before this PR's changes

@azat azat self-assigned this Jun 23, 2026
@clickhouse-gh

clickhouse-gh Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 85.40% 85.40% +0.00%
Functions 92.60% 92.60% +0.00%
Branches 77.60% 77.60% +0.00%

Changed lines: Changed C/C++ lines covered by tests: 3/3 (100.00%) | Lost baseline coverage: none · Uncovered code

Full report · Diff report

@vitlibar vitlibar added this pull request to the merge queue Jun 24, 2026
Merged via the queue into ClickHouse:master with commit 834059f Jun 24, 2026
328 of 330 checks passed
@vitlibar vitlibar deleted the fix-http-handler-header-regex-missing-header branch June 24, 2026 14:55
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@azat azat azat approved these changes

Assignees

@azat azat

Labels

pr-bugfix Pull request with bugfix, not backported by default pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vitlibar @azat @robot-clickhouse-ci-2