Skip to content

fix(terminal): Ctrl+Click does not open URLs ending with a port number#552

Merged
lzwind merged 1 commit into
linuxdeepin:masterfrom
st0nie:fix/url-ctrl-click-port
Jul 16, 2026
Merged

fix(terminal): Ctrl+Click does not open URLs ending with a port number#552
lzwind merged 1 commit into
linuxdeepin:masterfrom
st0nie:fix/url-ctrl-click-port

Conversation

@st0nie

@st0nie st0nie commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Problem

In deepin-terminal, Ctrl + Left-click silently fails to open a URL when the URL ends with a port number and no trailing path/slash:

URL Hover underline Ctrl+Click opens?
http://localhost:8000 ✅ shown does nothing
http://localhost:8000/ ✅ shown ✅ works
http://www.baidu.com:4443/ ✅ shown ✅ works
http://localhost:8000/path ✅ shown ⚠️ opens but truncated to http://localhost:8000/

So the user sees a working underline on http://localhost:8000, clicks it, and nothing happens — confusing and looks broken.

Root Cause

UrlFilter::FullUrlRegExp in 3rdparty/terminalwidget/lib/Filter.cpp validates the port number with a consuming boundary check, [^0-9]:

// before
"([:]((6553[0-5])|...|([0-9]))[^0-9])?"
//                                   ^^^^^ consuming: eats the next char into the URL

[^0-9] matches and consumes the first character after the port (the newline at end of line, a space, or a /). That character becomes part of the captured URL string. For http://localhost:8000 printed on its own line, the capture is http://localhost:8000\n.

UrlFilter::HotSpot::activate() then hands the string to QUrl(url, QUrl::StrictMode), which rejects any URL containing a control character like \n. The open action is dropped silently — no error, no browser.

The trailing-slash case http://localhost:8000/ happens to work because the consumed / still leaves a syntactically valid URL.

The same consumed / also explains the truncation in the last row: the optional path group ([/][\w.@?^=%&/~+#-]+)? can no longer match because the leading / was already eaten by [^0-9].

Fix

Replace the consuming character class [^0-9] with a non-consuming negative lookahead (?![0-9]):

// after
"([:]((6553[0-5])|...|([0-9]))(?![0-9]))?"
//                                   ^^^^^^^^ zero-width: asserts, does not consume

The lookahead asserts the port is not followed by another digit (so 80000 is still rejected as an out-of-range port) without swallowing the boundary character. The captured URL stays clean and QUrl::StrictMode accepts it. As a side effect, the trailing-path case also stops being truncated because the / is left available for the path group.

Verification

Tested with a Python equivalent of the regex (PCRE2-compatible, which is what QRegularExpression uses):

Input (buffer) Before (captured) After (captured)
echo http://localhost:8000\n (the bug) http://localhost:8000\n http://localhost:8000
echo http://localhost:8000/ \n http://localhost:8000/ http://localhost:8000
echo http://localhost:8000/path\n http://localhost:8000/ ❌ truncated http://localhost:8000/path
echo http://www.baidu.com:4443/\n http://www.baidu.com:4443/ http://www.baidu.com:4443
echo http://localhost:80000\n (invalid port) http://localhost (boundary still rejects) http://localhost (boundary still rejects) ✅

Invalid-port handling (80000, out-of-range 0–65535) is preserved — the lookahead still fails to match and the port group is dropped, identical to the previous behavior.

Scope of Change

A single one-character-class replacement in 3rdparty/terminalwidget/lib/Filter.cpp. No behavior change for valid URLs without ports, for email addresses, or for invalid port numbers.

How to Reproduce

  1. echo http://localhost:8000 in deepin-terminal
  2. Hover the URL — underline appears
  3. Ctrl + Left-click — nothing opens (bug)
  4. Compare with echo http://localhost:8000/ — Ctrl+Click opens the browser

After this patch, step 3 opens the browser as expected.

FullUrlRegExp's port group used a consuming boundary check [^0-9],
which swallowed the character immediately after the port (newline, space,
or '/') into the captured URL string. For URLs like 'http://localhost:8000'
at end of a line, the capture became 'http://localhost:8000\n' and
QUrl::StrictMode rejected it, so Ctrl+Click silently did nothing while
the hover underline still showed.

Replace [^0-9] with the non-consuming negative lookahead (?![0-9]).
The boundary character is now asserted but not consumed, so the captured
URL stays clean and QUrl accepts it. This also fixes a related issue
where 'http://localhost:8000/path' was truncated to 'http://localhost:8000/'
because the consumed '/' broke the optional path group.

Verified against: http://localhost:8000 (was broken), http://localhost:8000/
(works), http://localhost:8000/path (was truncated), http://www.baidu.com:4443/,
and invalid port boundary http://localhost:80000 (still rejected as expected).

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @st0nie, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@github-actions

Copy link
Copy Markdown

CLA Assistant Lite bot:
提交邮箱中包含我们的合作伙伴,但您似乎并非合作伙伴的成员或对接人,请联系相关对接人将您添加至组织之中,或由其重新发起 Pull Request。
The commit email domain belongs to one of our partners, but it seems you are not yet a member of the current organization, please contact the contact person to add you to the organization or let them submit the Pull Request.

You can retrigger this bot by commenting recheck in this Pull Request

@deepin-ci-robot

Copy link
Copy Markdown

Hi @st0nie. Thanks for your PR. 😃

@deepin-ci-robot

Copy link
Copy Markdown

Hi @st0nie. Thanks for your PR.

I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, st0nie

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@lzwind
lzwind merged commit f270534 into linuxdeepin:master Jul 16, 2026
15 of 16 checks passed
@st0nie
st0nie deleted the fix/url-ctrl-click-port branch July 16, 2026 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants