Skip to content

Commit b0ae573

Browse files
pombredannepombredanne
committed
Harden NPM repo urls handling #785
* ensure that url parsing utils do not choke on empty * do not populate repo fields in Package if the url is not presemt Signed-off-by: Philippe Ombredanne <[email protected]>
1 parent 01df534 commit b0ae573

File tree

6 files changed

+337
-4
lines changed

6 files changed

+337
-4
lines changed

‎src/packagedcode/npm.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,10 @@ def repository_mapper(repo, package):
323323
if isinstance(repo, basestring):
324324
package.vcs_repository = parse_repo_url(repo)
325325
elif isinstance(repo, dict):
326-
package.vcs_tool = repo.get('type') or 'git'
327-
package.vcs_repository = parse_repo_url(repo.get('url'))
326+
repurl = parse_repo_url(repo.get('url'))
327+
if repurl:
328+
package.vcs_tool = repo.get('type') or 'git'
329+
package.vcs_repository = repurl
328330
return package
329331

330332

‎src/packagedcode/utils.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,18 @@ def parse_repo_url(repo_url):
6767
https://gitlab.com/foo/private.git
6868
[email protected]:foo/private.git
6969
"""
70+
if not repo_url or not isinstance(repo_url, basestring):
71+
return
7072

73+
repo_url = repo_url.strip()
74+
if not repo_url:
75+
return
76+
77+
# TODO: If we match http and https, we may should add more check in
78+
# case if the url is not a repo one. For example, check the domain
79+
# name in the url...
7180
is_vcs_url = repo_url.startswith(VCS_URLS)
7281
if is_vcs_url:
73-
# TODO: If we match http and https, we may should add more check in case if the url is not a repo one.
74-
# For example, check the domain name in the url...
7582
return repo_url
7683

7784
if repo_url.startswith('git@'):

‎tests/packagedcode/data/npm/repo_url/package.json‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
{
2+
"name": "react-mobile-navigation-modal",
3+
"version": "0.1.0-rc.3",
4+
"description": "React mobile navigation modal.",
5+
"author": {
6+
"name": "Rostyslav Khanas",
7+
"email": "[email protected]"
8+
},
9+
"repository": {
10+
"type": "git"
11+
},
12+
"scripts": {
13+
"clean": "rimraf lib",
14+
"lint": "eslint src",
15+
"build": "npm run lint && npm run clean && babel src --out-dir lib"
16+
},
17+
"files": [
18+
"lib"
19+
],
20+
"main": "lib/index.js",
21+
"dependencies": {
22+
"binary-ui-icons": "^0.1.0-rc.8",
23+
"binary-ui-stack": "^0.0.1-rc.4",
24+
"binary-ui-styles": "^0.1.0-rc.8",
25+
"invariant": "^2.2.1",
26+
"react-mobile-navigation-core": "^0.1.0-rc.3",
27+
"react-mobile-navigation-engine": "^0.1.0-rc.3"
28+
},
29+
"peerDependencies": {
30+
"react": "~15.3.2"
31+
},
32+
"devDependencies": {
33+
"babel": "^6.3.26",
34+
"babel-cli": "^6.7.7",
35+
"babel-core": "^6.7.7",
36+
"babel-eslint": "^6.0.3",
37+
"babel-loader": "^6.2.1",
38+
"babel-plugin-transform-decorators-legacy": "^1.3.4",
39+
"babel-plugin-transform-object-rest-spread": "^6.5.0",
40+
"babel-preset-es2015": "^6.3.13",
41+
"babel-preset-es2015-allow-top-level-this": "0.0.1",
42+
"babel-preset-react": "^6.3.13",
43+
"eslint": "^2.5.3",
44+
"eslint-config-airbnb": "^9.0.1",
45+
"eslint-plugin-import": "^1.5.0",
46+
"eslint-plugin-jsx-a11y": "^1.0.2",
47+
"eslint-plugin-react": "^5.0.1",
48+
"node-uuid": "^1.4.7",
49+
"react": "~15.3.2",
50+
"react-art": "^0.15.1",
51+
"react-dom": "~15.3.2",
52+
"react-motion": "^0.4.7",
53+
"react-redux": "^4.0.6",
54+
"redux": "^3.0.5",
55+
"rimraf": "^2.3.4",
56+
"webpack": "^1.12.15",
57+
"webpack-dev-server": "^1.14.1"
58+
},
59+
"_id": "[email protected]",
60+
"_shasum": "0ae0b2065295b14c9db60852215b3f60d5510a99",
61+
"_from": ".",
62+
"_npmVersion": "4.3.0",
63+
"_nodeVersion": "7.4.0",
64+
"_npmUser": {
65+
"name": "rtkhanas",
66+
"email": "[email protected]"
67+
},
68+
"dist": {
69+
"shasum": "0ae0b2065295b14c9db60852215b3f60d5510a99",
70+
"tarball": "https://registry.npmjs.org/react-mobile-navigation-modal/-/react-mobile-navigation-modal-0.1.0-rc.3.tgz"
71+
},
72+
"maintainers": [
73+
{
74+
"name": "rtkhanas",
75+
"email": "[email protected]"
76+
}
77+
],
78+
"_npmOperationalInternal": {
79+
"host": "packages-18-east.internal.npmjs.com",
80+
"tmp": "tmp/react-mobile-navigation-modal-0.1.0-rc.3.tgz_1489492444412_0.4683970238547772"
81+
},
82+
"directories": {}
83+
}

‎tests/packagedcode/data/npm/repo_url/package.json.expected‎

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
{
2+
"type": "npm",
3+
"name": "react-mobile-navigation-modal",
4+
"version": "0.1.0-rc.3",
5+
"primary_language": "JavaScript",
6+
"packaging": null,
7+
"summary": "React mobile navigation modal.",
8+
"description": null,
9+
"payload_type": null,
10+
"size": null,
11+
"release_date": null,
12+
"authors": [
13+
{
14+
"type": "person",
15+
"name": "Rostyslav Khanas",
16+
"email": "[email protected]",
17+
"url": null
18+
}
19+
],
20+
"maintainers": [
21+
{
22+
"type": "person",
23+
"name": "rtkhanas",
24+
"email": "[email protected]",
25+
"url": null
26+
}
27+
],
28+
"contributors": [],
29+
"owners": [],
30+
"packagers": [],
31+
"distributors": [],
32+
"vendors": [],
33+
"keywords": [],
34+
"keywords_doc_url": null,
35+
"metafile_locations": [
36+
"package"
37+
],
38+
"metafile_urls": [],
39+
"homepage_url": null,
40+
"notes": null,
41+
"download_urls": [
42+
"https://registry.npmjs.org/react-mobile-navigation-modal/-/react-mobile-navigation-modal-0.1.0-rc.3.tgz"
43+
],
44+
"download_sha1": "0ae0b2065295b14c9db60852215b3f60d5510a99",
45+
"download_sha256": null,
46+
"download_md5": null,
47+
"bug_tracking_url": null,
48+
"support_contacts": [],
49+
"code_view_url": null,
50+
"vcs_tool": null,
51+
"vcs_repository": null,
52+
"vcs_revision": null,
53+
"copyright_top_level": null,
54+
"copyrights": [],
55+
"asserted_licenses": [],
56+
"legal_file_locations": [],
57+
"license_expression": null,
58+
"license_texts": [],
59+
"notice_texts": [],
60+
"dependencies": {
61+
"development": [
62+
{
63+
"name": "babel",
64+
"version": null,
65+
"version_constraint": "^6.3.26"
66+
},
67+
{
68+
"name": "babel-cli",
69+
"version": null,
70+
"version_constraint": "^6.7.7"
71+
},
72+
{
73+
"name": "babel-core",
74+
"version": null,
75+
"version_constraint": "^6.7.7"
76+
},
77+
{
78+
"name": "babel-eslint",
79+
"version": null,
80+
"version_constraint": "^6.0.3"
81+
},
82+
{
83+
"name": "babel-loader",
84+
"version": null,
85+
"version_constraint": "^6.2.1"
86+
},
87+
{
88+
"name": "babel-plugin-transform-decorators-legacy",
89+
"version": null,
90+
"version_constraint": "^1.3.4"
91+
},
92+
{
93+
"name": "babel-plugin-transform-object-rest-spread",
94+
"version": null,
95+
"version_constraint": "^6.5.0"
96+
},
97+
{
98+
"name": "babel-preset-es2015",
99+
"version": null,
100+
"version_constraint": "^6.3.13"
101+
},
102+
{
103+
"name": "babel-preset-es2015-allow-top-level-this",
104+
"version": null,
105+
"version_constraint": "0.0.1"
106+
},
107+
{
108+
"name": "babel-preset-react",
109+
"version": null,
110+
"version_constraint": "^6.3.13"
111+
},
112+
{
113+
"name": "eslint",
114+
"version": null,
115+
"version_constraint": "^2.5.3"
116+
},
117+
{
118+
"name": "eslint-config-airbnb",
119+
"version": null,
120+
"version_constraint": "^9.0.1"
121+
},
122+
{
123+
"name": "eslint-plugin-import",
124+
"version": null,
125+
"version_constraint": "^1.5.0"
126+
},
127+
{
128+
"name": "eslint-plugin-jsx-a11y",
129+
"version": null,
130+
"version_constraint": "^1.0.2"
131+
},
132+
{
133+
"name": "eslint-plugin-react",
134+
"version": null,
135+
"version_constraint": "^5.0.1"
136+
},
137+
{
138+
"name": "node-uuid",
139+
"version": null,
140+
"version_constraint": "^1.4.7"
141+
},
142+
{
143+
"name": "react",
144+
"version": null,
145+
"version_constraint": "~15.3.2"
146+
},
147+
{
148+
"name": "react-art",
149+
"version": null,
150+
"version_constraint": "^0.15.1"
151+
},
152+
{
153+
"name": "react-dom",
154+
"version": null,
155+
"version_constraint": "~15.3.2"
156+
},
157+
{
158+
"name": "react-motion",
159+
"version": null,
160+
"version_constraint": "^0.4.7"
161+
},
162+
{
163+
"name": "react-redux",
164+
"version": null,
165+
"version_constraint": "^4.0.6"
166+
},
167+
{
168+
"name": "redux",
169+
"version": null,
170+
"version_constraint": "^3.0.5"
171+
},
172+
{
173+
"name": "rimraf",
174+
"version": null,
175+
"version_constraint": "^2.3.4"
176+
},
177+
{
178+
"name": "webpack",
179+
"version": null,
180+
"version_constraint": "^1.12.15"
181+
},
182+
{
183+
"name": "webpack-dev-server",
184+
"version": null,
185+
"version_constraint": "^1.14.1"
186+
}
187+
],
188+
"runtime": [
189+
{
190+
"name": "binary-ui-icons",
191+
"version": null,
192+
"version_constraint": "^0.1.0-rc.8"
193+
},
194+
{
195+
"name": "binary-ui-stack",
196+
"version": null,
197+
"version_constraint": "^0.0.1-rc.4"
198+
},
199+
{
200+
"name": "binary-ui-styles",
201+
"version": null,
202+
"version_constraint": "^0.1.0-rc.8"
203+
},
204+
{
205+
"name": "invariant",
206+
"version": null,
207+
"version_constraint": "^2.2.1"
208+
},
209+
{
210+
"name": "react-mobile-navigation-core",
211+
"version": null,
212+
"version_constraint": "^0.1.0-rc.3"
213+
},
214+
{
215+
"name": "react-mobile-navigation-engine",
216+
"version": null,
217+
"version_constraint": "^0.1.0-rc.3"
218+
}
219+
],
220+
"optional": [
221+
{
222+
"name": "react",
223+
"version": null,
224+
"version_constraint": "~15.3.2"
225+
}
226+
]
227+
},
228+
"related_packages": []
229+
}

‎tests/packagedcode/test_npm.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,3 +135,10 @@ def test_parse_from_urls_dict_legacy_is_ignored(self):
135135
package = npm.parse(test_file)
136136
self.check_package(package, expected_loc, regen=False, fix_locations=False)
137137
package.validate()
138+
139+
def test_parse_does_not_crash_if_partial_repo_url(self):
140+
test_file = self.get_test_loc('npm/repo_url/package.json')
141+
expected_loc = self.get_test_loc('npm/repo_url/package.json.expected')
142+
package = npm.parse(test_file)
143+
self.check_package(package, expected_loc, regen=False, fix_locations=False)
144+
package.validate()

‎tests/packagedcode/test_package_utils.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,8 @@ def test_parse_git_repo_url_without_slash_slash(self):
106106
test = '[email protected]/Filirom1/npm2aur.git'
107107
expected = 'https://github.com/Filirom1/npm2aur.git'
108108
assert expected == parse_repo_url(test)
109+
110+
def test_parse_repo_url_does_not_fail_on_empty(self):
111+
assert None == parse_repo_url(None)
112+
assert None == parse_repo_url('')
113+
assert None == parse_repo_url(' ')

0 commit comments

Comments
 (0)