Skip to content

Commit be5cce3

Browse files
committed
Encode the issue title like the body
The URL the tray opens after an error put the raw message in title= while encoding body=. The message carries a file path, so a '#' in one - a directory named C# will do it - started a fragment: the body went, and the title ended at the hash. An '&' started a parameter GitHub does not have, truncating the title there. Both halves are built in one place now, and the title goes through UrlEncode. The body still arrives already encoded from its callers, which is why it is appended rather than encoded again.
1 parent ed53e24 commit be5cce3

2 files changed

Lines changed: 60 additions & 5 deletions

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/// <summary>
2+
/// The issue URL the tray opens after an error. Its title is built from a message that carries a
3+
/// file path, so it holds whatever characters the path does.
4+
/// </summary>
5+
public class IssueLauncherTests
6+
{
7+
[Test]
8+
public async Task A_hash_in_the_title_does_not_start_a_fragment()
9+
{
10+
// Perfectly ordinary in a solution directory, and everything after it used to be read by
11+
// the browser as a fragment: no body, and a title ending at the hash
12+
const string message = @"Cannot start. Failed to read settings: C:\code\C#\settings.json";
13+
14+
var url = IssueLauncher.BuildUrl(message);
15+
16+
await Assert.That(url).DoesNotContain("#");
17+
await Assert.That(Query(url)["title"]).IsEqualTo(message);
18+
await Assert.That(Query(url)["body"]).IsNotEmpty();
19+
}
20+
21+
[Test]
22+
public async Task An_ampersand_in_the_title_does_not_truncate_it()
23+
{
24+
const string message = @"Could not accept 'R&D.received.txt'";
25+
26+
var url = IssueLauncher.BuildUrl(message);
27+
28+
await Assert.That(Query(url)["title"]).IsEqualTo(message);
29+
await Assert.That(Query(url)["body"]).IsNotEmpty();
30+
}
31+
32+
/// <summary>
33+
/// The body is encoded by its callers and handed over already escaped, so encoding the title
34+
/// must not have changed what reaches GitHub as the body.
35+
/// </summary>
36+
[Test]
37+
public async Task Keeps_the_body_it_is_given()
38+
{
39+
var url = IssueLauncher.BuildUrl("TheTitle", WebUtility.UrlEncode("\n * Action: TheAction"));
40+
41+
await Assert.That(Query(url)["body"]).Contains("* Action: TheAction");
42+
}
43+
44+
static Dictionary<string, string> Query(string url) =>
45+
url[(url.IndexOf('?') + 1)..]
46+
.Split('&')
47+
.Select(_ => _.Split('=', 2))
48+
.ToDictionary(_ => _[0], _ => WebUtility.UrlDecode(_[1]));
49+
}

‎src/DiffEngineTray/IssueLauncher.cs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,15 @@ static IssueLauncher() =>
1111
""");
1212

1313
public static void Launch() =>
14-
LinkLauncher.LaunchUrl($"https://github.com/VerifyTests/DiffEngine/issues/new?title=TODO&body={defaultBody}");
14+
LinkLauncher.LaunchUrl(BuildUrl("TODO"));
15+
16+
/// <summary>
17+
/// The title is encoded like the body is. It is built from a message carrying a file path, and
18+
/// a '#' in one started a fragment - dropping the body, and everything of the title after it -
19+
/// while an '&amp;' started a parameter GitHub does not have, truncating the title there.
20+
/// </summary>
21+
internal static string BuildUrl(string title, string extraBody = "") =>
22+
$"https://github.com/VerifyTests/DiffEngine/issues/new?title={WebUtility.UrlEncode(title)}&body={defaultBody}{extraBody}";
1523

1624
public static void LaunchForException(string message, Exception exception)
1725
{
@@ -43,8 +51,7 @@ Open an issue on GitHub?
4351
{exception}
4452
```
4553
""");
46-
var url = $"https://github.com/VerifyTests/DiffEngine/issues/new?title={message}&body={defaultBody}{extraBody}";
47-
LinkLauncher.LaunchUrl(url);
54+
LinkLauncher.LaunchUrl(BuildUrl(message, extraBody));
4855
}
4956

5057
public static void LaunchForException(string message)
@@ -71,8 +78,7 @@ Open an issue on GitHub?
7178
7279
* Action: {message}
7380
""");
74-
var url = $"https://github.com/VerifyTests/DiffEngine/issues/new?title={message}&body={defaultBody}{extraBody}";
75-
LinkLauncher.LaunchUrl(url);
81+
LinkLauncher.LaunchUrl(BuildUrl(message, extraBody));
7682
}
7783

7884
static bool AskIfOpenIssue(string text)

0 commit comments

Comments
 (0)