fix: handle quoted charset and non-canonical content-type in title extraction - #2591
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. WalkthroughThe change normalizes MIME types before title detection and removes quotes from content-type values before charset decoding. Tests cover MIME casing, whitespace, and GBK title decoding with quoted and unquoted charset parameters. ChangesHTTP title extraction
Merge Risk: ⚪ Minimal · up to Title extraction now handles non-canonical Content-Type formatting and quoted GBK charset values while retaining canonical behavior. The covered changes present no remaining merge-readiness risk. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ehsandeep
left a comment
There was a problem hiding this comment.
@krishna28238-arch thanks for the PR, do you have test target to see before / after behavior?
|
Yes — I've included end-to-end regression tests that Test 1: TestCanHaveTitleTag (title_test.go) Test 2: TestExtractTitleDecodesCharset (title_test.go) You can verify the before/after behavior by:
Or run the full test suite to confirm no regressions: |
|
@krishna28238-arch I'm looking for real hosts to better understand the existing tool behavior, then test code to better understand the change. Can you share some? |
|
Thanks @ehsandeep, sure - here's what I found. The short version: the mainstream CJK web has moved to canonical UTF-8 headers, the legacy encodings are still out there, and the two shapes this PR fixes are straightforward to reproduce locally even though I couldn't find a live public host serving them today. Real hosts I checked I ran (baidu.com, qq.com, 163.com, sohu.com, sina.com.cn, gov.cn, people.com.cn, cctv.com, china.com.cn, naver.com, daum.net, yahoo.co.jp, etc) One host that is still GB2312 today, in case you want to watch the existing behavior end to end: dev gets this one right - the So the meta-declared path works today. What doesn't work is when the same value comes back in the HTTP header in non-canonical form:
This same class of host has come up in the tracker a few times already: #37 (tnw.qq.com, GBK, 2020), #441 (euc-kr titles, 2021), #652 (GBK page, 2022), #1494 (103.45.249.230:7878, 2023). All four are migrated or offline now, so for the code-test part I set up local servers that emit the exact shapes: # save as weird_ct.py
from http.server import HTTPServer, BaseHTTPRequestHandler
import threading, signal
class Handler(BaseHTTPRequestHandler):
body = b''
ctype = ''
def do_GET(self):
self.send_response(200)
self.send_header('Content-Type', self.ctype)
self.send_header('Content-Length', str(len(self.body)))
self.end_headers()
self.wfile.write(self.body)
def log_message(self, fmt, *args):
pass
class QuotedGBK(Handler):
ctype = 'text/html; charset="gbk"'
body = '<html><head><title>这是中文测试标题</title></head><body>Content</body></html>'.encode('gbk')
class UppercaseMIME(Handler):
ctype = 'TEXT/HTML'
body = b'<html><head><title>Uppercase MIME Type Test</title></head><body>Content</body></html>'
class Whitespace(Handler):
ctype = 'text/html ; charset=utf-8' # note the space before the semicolon
body = b'<html><head><title>Whitespace Test</title></head><body>Content</body></html>'
for port, h in [(8001, QuotedGBK), (8002, UppercaseMIME), (8003, Whitespace)]:
threading.Thread(target=HTTPServer(('127.0.0.1', port), h).serve_forever, daemon=True).start()
signal.pause()dev branch: and to confirm the first one is really just undecoded GBK (the same bytes the server sent): with this PR: Regression check I ran both builds over the 32 reachable hosts from the scan with |
httpx silently drops the title, or extracts it as mojibake, when the
Content-Typeresponse header deviates from the canonical lowercase form:Content-Type: text/html; charset="gbk"— the quoted charset value (legal per RFC 9110) is not matched by thecharset=gbksubstring check inDecodeData, so GBK/EUC-KR bodies are never transcoded and the title comes out as raw bytesContent-Type: TEXT/HTMLortext/html ; charset=utf-8— media types are case-insensitive and optional whitespace is allowed before parameters, butCanHaveTitleTagcompares the value verbatim, so the title extraction is skipped entirelyThis strips quotes from the joined content-type before the charset matching, and normalizes the mime type (case + surrounding whitespace) in
CanHaveTitleTag. Responses with canonical headers are unaffected. Regression tests cover both the quoted-charset and case/whitespace variants end-to-end (TestCanHaveTitleTag,TestExtractTitleDecodesCharset).Summary by CodeRabbit
Bug Fixes
Tests