fix(fmt): stop share-URI query values from being decoded twice or dropped (#10027)

`Utils.ParseQueryString` already unescapes every value, and
`BaseFmt.GetQueryDecoded` unescaped it again. A value that still held a
valid percent sequence after the first pass decayed on the second: an
obfuscation password of `ob%41fs` is exported as `ob%2541fs` and imported
back as `obAfs`. Only well-formed sequences are affected, which is why the
damage is silent - `100%` and `66%ff` survive untouched.

The same function also split each pair on every `=`, and skipped the pair
unless exactly two halves came out. RFC 3986 lists `=` among the
sub-delimiters a query value may carry, so only the first one separates the
key from the value, and `HttpUtility.ParseQueryString` reads a query string
the same way. Splitting on all of them discarded a syntactically valid
pair: `?ech=AAj+DQAEAAAAAA==` was lost entirely, and so was a `plugin`
value in the non-canonical SIP002 spelling, since those are `;` separated
`key=value` lists.

v2rayN percent-encodes both on export, so its own links were never
affected; what changes is that the parser now follows the grammar instead
of discarding a pair it cannot split in two.

Splitting on the first `=` only, and reading the value the parser already
decoded, fixes both. `ParseQueryString` keeps decoding because
`ConfigHandler` reads its result directly.

`GetQueryDecoded` and `GetQueryValue` are now equivalent; they are left
separate to keep this change small, and can be collapsed if you prefer.
This commit is contained in:
Miheichev Aleksandr Sergeevich
2026-08-28 06:38:02 +00:00
committed by GitHub
parent f332caf507
commit 2a65e5ac80
3 changed files with 82 additions and 2 deletions

View File

@@ -0,0 +1,73 @@
namespace ServiceLib.Tests.Fmt;
public class ShareUriQueryTests
{
[Test]
[Arguments("ob%41fs")]
[Arguments("66%ff")]
[Arguments("100%")]
public async Task GetShareUriAndResolveConfig_QueryValueWithPercent_ShouldSurviveTheRoundTrip(string obfsPassword)
{
var source = new ProfileItem
{
ConfigType = EConfigType.Hysteria2,
Remarks = "percent demo",
Address = "hy2.example",
Port = 8443,
Password = "pw",
};
source.SetProtocolExtra(new ProtocolExtraItem { SalamanderPass = obfsPassword, });
var uri = FmtHandler.GetShareUri(source);
await uri.Should().NotBeNull();
var resolved = FmtHandler.ResolveConfig(uri!, out var msg);
await resolved.Should().NotBeNull().Because($"uri: {uri}, msg: {msg}");
await resolved!.GetProtocolExtra().SalamanderPass.Should().BeEqualTo(obfsPassword);
}
// RFC 3986 lists '=' among the sub-delimiters a query value may carry, so only the first one
// separates the key from the value. Splitting on every '=' discarded the pair outright.
[Test]
public async Task ResolveConfig_QueryValueWithUnescapedEquals_ShouldNotBeDropped()
{
const string shareUri = "hysteria2://pw@hy2.example:8443/?ech=AAj+DQAEAAAAAA==&sni=real.example";
var resolved = FmtHandler.ResolveConfig(shareUri, out var msg);
await resolved.Should().NotBeNull().Because($"uri: {shareUri}, msg: {msg}");
await resolved!.EchConfigList.Should().BeEqualTo("AAj+DQAEAAAAAA==");
await resolved.Sni.Should().BeEqualTo("real.example");
}
// Canonical SIP002 percent-encodes the plugin argument, and that is what this client's own
// exporter emits. This covers the non-canonical spelling instead: the options are a ';'
// separated list of 'key=value' pairs, so left unescaped the value always carries '='.
[Test]
public async Task ResolveConfig_NonCanonicalSip002PluginWithLiteralEquals_ShouldConfigureObfs()
{
const string shareUri =
"ss://YWVzLTEyOC1nY206cGFzczEyMw==@1.2.3.4:8388/?plugin=obfs-local;obfs=http;obfs-host=example.com#ss";
var resolved = FmtHandler.ResolveConfig(shareUri, out var msg);
await resolved.Should().NotBeNull().Because($"uri: {shareUri}, msg: {msg}");
await resolved!.ConfigType.Should().BeEqualTo(EConfigType.Shadowsocks);
await resolved.GetTransportExtra().Host.Should().BeEqualTo("example.com");
await resolved.GetTransportExtra().RawHeaderType.Should().BeEqualTo(Global.RawHeaderHttp);
}
[Test]
public async Task ResolveConfig_EscapedQueryValue_ShouldStillDecodeExactlyOnce()
{
const string shareUri = "hysteria2://pw@hy2.example:8443/?ech=AAj%2BDQAEAAAAAA%3D%3D&obfs=salamander&obfs-password=a%20b";
var resolved = FmtHandler.ResolveConfig(shareUri, out var msg);
await resolved.Should().NotBeNull().Because($"uri: {shareUri}, msg: {msg}");
await resolved!.EchConfigList.Should().BeEqualTo("AAj+DQAEAAAAAA==");
await resolved.GetProtocolExtra().SalamanderPass.Should().BeEqualTo("a b");
}
}

View File

@@ -200,7 +200,9 @@ public class Utils
var parts = query[1..].Split('&', StringSplitOptions.RemoveEmptyEntries);
foreach (var part in parts)
{
var keyValue = part.Split('=');
// Split on the FIRST '=' only: RFC 3986 lists '=' among the sub-delimiters a query
// value may carry, so everything after the first one belongs to the value.
var keyValue = part.Split('=', 2);
if (keyValue.Length != 2)
{
continue;

View File

@@ -342,8 +342,13 @@ public class BaseFmt
return query[key] ?? defaultValue;
}
/// <summary>
/// Values are already unescaped by <see cref="Utils.ParseQueryString" />, so this must not
/// unescape them a second time: a value that still holds a valid percent sequence after the
/// first pass - an obfuscation password of "ob%41fs", say - would decay into "obAfs".
/// </summary>
protected static string GetQueryDecoded(NameValueCollection query, string key, string defaultValue = "")
{
return Utils.UrlDecode(GetQueryValue(query, key, defaultValue));
return GetQueryValue(query, key, defaultValue);
}
}