Skip to content

Commit 2598d69

Browse files
authored
Merge pull request LykosAI#1561 from e-nord/fix-pip-show-results-crash
Fix: Avoid duplicate pip package entries in results
2 parents 73eac4d + b1292d0 commit 2598d69

2 files changed

Lines changed: 215 additions & 8 deletions

File tree

StabilityMatrix.Core/Python/PipShowResult.cs

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,39 @@ public static PipShowResult Parse(string output)
4545
lines.RemoveRange(indexOfLicense, indexOfLocation - indexOfLicense);
4646
}
4747

48-
var linesDict = lines
49-
.Select(line => line.Split(':', 2))
50-
.Where(split => split.Length == 2)
51-
.Select(split => new KeyValuePair<string, string>(split[0].Trim(), split[1].Trim()))
52-
.ToDictionary(pair => pair.Key, pair => pair.Value);
48+
var linesDict = new Dictionary<string, string>();
49+
foreach (var line in lines)
50+
{
51+
var split = line.Split(':', 2);
52+
if (split.Length != 2)
53+
continue;
54+
55+
var key = split[0].Trim();
56+
var value = split[1].Trim();
57+
58+
if (key == "Name" && linesDict.ContainsKey("Name"))
59+
{
60+
// We've hit a new package, so stop parsing
61+
break;
62+
}
63+
64+
linesDict.TryAdd(key, value);
65+
}
66+
67+
if (!linesDict.TryGetValue("Name", out var name))
68+
{
69+
throw new FormatException("The 'Name' key was not found in the pip show output.");
70+
}
71+
72+
if (!linesDict.TryGetValue("Version", out var version))
73+
{
74+
throw new FormatException("The 'Version' key was not found in the pip show output.");
75+
}
5376

5477
return new PipShowResult
5578
{
56-
Name = linesDict["Name"],
57-
Version = linesDict["Version"],
79+
Name = name,
80+
Version = version,
5881
Summary = linesDict.GetValueOrDefault("Summary"),
5982
HomePage = linesDict.GetValueOrDefault("Home-page"),
6083
Author = linesDict.GetValueOrDefault("Author"),
@@ -68,7 +91,7 @@ public static PipShowResult Parse(string output)
6891
RequiredBy = linesDict
6992
.GetValueOrDefault("Required-by")
7093
?.Split(',', StringSplitOptions.TrimEntries)
71-
.ToList()
94+
.ToList(),
7295
};
7396
}
7497

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
using System.IO;
2+
using System.Threading.Tasks;
3+
using Microsoft.VisualStudio.TestTools.UnitTesting;
4+
using StabilityMatrix.Core.Python;
5+
6+
namespace StabilityMatrix.Tests.Core;
7+
8+
[TestClass]
9+
public class PipShowResultsTests
10+
{
11+
[TestMethod]
12+
public void TestSinglePackage()
13+
{
14+
var input = """
15+
Name: package-a
16+
Version: 1.0.0
17+
Summary: A test package
18+
Home-page: https://example.com
19+
Author: John Doe
20+
Author-email: john.doe@example.com
21+
License: MIT
22+
Location: /path/to/package
23+
Requires:
24+
Required-by:
25+
""";
26+
27+
var result = PipShowResult.Parse(input);
28+
29+
Assert.IsNotNull(result);
30+
Assert.AreEqual("package-a", result.Name);
31+
Assert.AreEqual("1.0.0", result.Version);
32+
Assert.AreEqual("A test package", result.Summary);
33+
}
34+
35+
[TestMethod]
36+
public void TestMultiplePackages()
37+
{
38+
var input = """
39+
Name: package-a
40+
Version: 1.0.0
41+
Summary: A test package
42+
Home-page: https://example.com
43+
Author: John Doe
44+
Author-email: john.doe@example.com
45+
License: MIT
46+
Location: /path/to/package
47+
Requires:
48+
Required-by:
49+
---
50+
Name: package-b
51+
Version: 2.0.0
52+
Summary: Another test package
53+
Home-page: https://example.com
54+
Author: Jane Doe
55+
Author-email: jane.doe@example.com
56+
License: Apache-2.0
57+
Location: /path/to/another/package
58+
Requires: package-a
59+
Required-by:
60+
""";
61+
62+
var result = PipShowResult.Parse(input);
63+
64+
Assert.IsNotNull(result);
65+
Assert.AreEqual("package-a", result.Name);
66+
Assert.AreEqual("1.0.0", result.Version);
67+
Assert.AreNotEqual("package-b", result.Name);
68+
}
69+
70+
[TestMethod]
71+
public void TestMalformedPackage()
72+
{
73+
var input = """
74+
Name: package-a
75+
Version: 1.0.0
76+
Summary A test package
77+
Home-page: https://example.com
78+
Author: John Doe
79+
Author-email: john.doe@example.com
80+
License: MIT
81+
Location: /path/to/package
82+
Requires:
83+
Required-by:
84+
""";
85+
86+
var result = PipShowResult.Parse(input);
87+
88+
Assert.IsNotNull(result);
89+
Assert.AreEqual("package-a", result.Name);
90+
Assert.AreEqual("1.0.0", result.Version);
91+
Assert.IsNull(result.Summary);
92+
}
93+
94+
[TestMethod]
95+
public void TestMultiLineLicense()
96+
{
97+
var input = """
98+
Name: package-a
99+
Version: 1.0.0
100+
Summary: A test package
101+
Home-page: https://example.com
102+
Author: John Doe
103+
Author-email: john.doe@example.com
104+
License: The MIT License (MIT)
105+
106+
Copyright (c) 2015 John Doe
107+
108+
Permission is hereby granted, free of charge, to any person obtaining a copy
109+
of this software and associated documentation files (the "Software"), to deal
110+
in the Software without restriction, including without limitation the rights
111+
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
112+
copies of the Software, and to permit persons to whom the Software is
113+
furnished to do so, subject to the following conditions:
114+
115+
The above copyright notice and this permission notice shall be included in all
116+
copies or substantial portions of the Software.
117+
118+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
119+
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
120+
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
121+
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
122+
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
123+
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
124+
SOFTWARE.
125+
Location: /path/to/package
126+
Requires:
127+
Required-by:
128+
""";
129+
130+
var result = PipShowResult.Parse(input);
131+
132+
Assert.IsNotNull(result);
133+
Assert.AreEqual("package-a", result.Name);
134+
Assert.AreEqual("1.0.0", result.Version);
135+
Assert.IsTrue(result.License?.StartsWith("License: The MIT License (MIT)"));
136+
}
137+
138+
/// <summary>
139+
/// This test simulates the input that caused the crash reported in Sentry issue b125504f.
140+
/// The old implementation of PipShowResult.Parse used ToDictionary, which would throw an
141+
/// ArgumentException if the input contained multiple packages, as the "Name" key would be
142+
/// duplicated. The new implementation uses a foreach loop and TryAdd to prevent this crash.
143+
/// </summary>
144+
[TestMethod]
145+
public void TestDuplicatePackageNameInOutput()
146+
{
147+
var input = """
148+
Name: package-a
149+
Version: 1.0.0
150+
Summary: A test package
151+
Home-page: https://example.com
152+
Author: John Doe
153+
Author-email: john.doe@example.com
154+
License: MIT
155+
Location: /path/to/package
156+
Requires:
157+
Required-by:
158+
---
159+
Name: package-a
160+
Version: 1.0.0
161+
Summary: A test package
162+
Home-page: https://example.com
163+
Author: John Doe
164+
Author-email: john.doe@example.com
165+
License: MIT
166+
Location: /path/to/package
167+
Requires:
168+
Required-by:
169+
""";
170+
171+
var result = PipShowResult.Parse(input);
172+
173+
Assert.IsNotNull(result);
174+
Assert.AreEqual("package-a", result.Name);
175+
Assert.AreEqual("1.0.0", result.Version);
176+
}
177+
178+
[TestMethod]
179+
public void TestEmptyInputThrowsFormatException()
180+
{
181+
var input = "";
182+
Assert.ThrowsException<FormatException>(() => PipShowResult.Parse(input));
183+
}
184+
}

0 commit comments

Comments
 (0)