fix(upnp): pin LC_CTYPE = C around ASCII tolower() in UPnPBase - #208
Merged
Merged
Conversation
UPnPBase.cpp has four call sites that rely on tolower() producing
ASCII semantics:
* stdStringIsEqualCI / stdStringStartsWithCI -- the two
case-insensitive std::string helpers used throughout the file.
* CUPnPService::Execute -- direct comparison against 'i' and 'n' to
validate the UPnP argument direction string ("in" / "out").
* CUPnPControlPoint::Callback BYEBYE case -- transforms the
discovered device-type URN to lowercase before matching against
UPnP::Device::IGW.
Under tr_TR.UTF-8 (and any other Turkish-locale variant), libc
tolower('I') returns U+0131 (dotless i) instead of 'i'. This
silently breaks UPnP direction validation, device-type matching,
namespace lowercase comparisons, and HTTP-header lowercasing --
amuled's UPnP auto-portforward stops working for Turkish-locale
users.
Before amule-project#205 (i18n: fix non-ASCII mangling under default C locale),
amuled stayed on the C locale throughout its lifetime and these
sites were silently safe. The fix in amule-project#205 made amuled pick LC_CTYPE
up from the environment so unicode2char() emits real UTF-8 -- which
exposed the latent Turkish-i issue documented above.
Wrap each tolower() scope in CCtypeAsciiScope -- the project's
existing RAII helper that pins LC_CTYPE = "C" for the duration of
its scope and restores it on destruction. Same pattern already used
in CamuleFileConfig.h (amule-project#852) and MaxMindDBDatabase.cpp.
amule (GUI) was already exposed to this bug via wxLocale's
setlocale(LC_ALL) on Turkish-language installs; the wrap fixes it
there too as a free side effect.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #205. After that PR landed,
amuledstarted pickingLC_CTYPEup from the environment instead of staying on the C locale. That's the right behaviour for non-ASCII filenames, but it exposed a latent Turkish-i issue inUPnPBase.cpp: fourtolower()call sites that rely on ASCII semantics and silently break undertr_TR.UTF-8(wheretolower('I')returnsı/ U+0131 instead ofi).Net effect for Turkish-locale users on
amuledwith UPnP enabled: auto-portforward silently fails becauseCUPnPService::Executerejects every argument's direction string.amule(GUI) was already exposed to the same bug viawxLocale::Init()on Turkish-language installs; this also fixes it there as a side effect.Sites wrapped
All four use
CCtypeAsciiScope(the existing RAII helper that pinsLC_CTYPE = "C"for the duration of its scope and restores it on destruction — same pattern asCamuleFileConfig.hfrom #852 andMaxMindDBDatabase.cppfrom the GeoIP flag fix):stdStringIsEqualCI— case-insensitivestd::stringhelper used throughout the file.stdStringStartsWithCI— sibling helper, same shape.CUPnPService::Execute— directtolower(direction[0]) != 'i'validation against "in"/"out".CUPnPControlPoint::CallbackBYEBYE case — device-type URN lowercase before matchingUPnP::Device::IGW.Test plan
-DENABLE_UPNP=YES(amule,amuled,amulecmd,amuleweball build clean; zero new errors, zero new warnings).