Skip to content

Commit a51b204

Browse files
mitchell-codecovmitchell-codecov
authored
Merge pull request #604 from codecov/sanitize-url-inputs
refactor: use url objects for input sanitization
2 parents 8fa5fe4 + 1752d93 commit a51b204

File tree

5 files changed

+100
-70
lines changed

5 files changed

+100
-70
lines changed

‎src/helpers/web.ts‎

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { version } from '../../package.json'
55
import {
66
IRequestHeaders,
77
IServiceParams,
8+
PostResults,
9+
PutResults,
810
UploaderArgs,
911
UploaderInputs,
1012
} from '../types'
@@ -49,17 +51,16 @@ export function getPackage(source: string): string {
4951
}
5052
}
5153

54+
5255
export async function uploadToCodecovPUT(
53-
uploadURL: string,
56+
putAndResultUrlPair: PostResults,
5457
uploadFile: string | Buffer,
5558
args: UploaderArgs
56-
): Promise<{ status: string; resultURL: string }> {
59+
): Promise<PutResults> {
5760
info('Uploading...')
5861

59-
const { putURL, resultURL } = parsePOSTResults(uploadURL)
60-
6162
const requestHeaders = generateRequestHeadersPUT(
62-
putURL,
63+
putAndResultUrlPair.putURL,
6364
uploadFile,
6465
args,
6566
)
@@ -72,18 +73,18 @@ export async function uploadToCodecovPUT(
7273
)
7374
}
7475

75-
return { status: 'success', resultURL }
76+
return { status: 'success', resultURL: putAndResultUrlPair.resultURL }
7677
}
7778

78-
export async function uploadToCodecov(
79-
uploadURL: string,
79+
export async function uploadToCodecovPOST(
80+
postURL: URL,
8081
token: string,
8182
query: string,
8283
source: string,
8384
args: UploaderArgs,
8485
): Promise<string> {
8586
const requestHeaders = generateRequestHeadersPOST(
86-
uploadURL,
87+
postURL,
8788
token,
8889
query,
8990
source,
@@ -112,17 +113,18 @@ export function generateQuery(queryParams: Partial<IServiceParams>): string {
112113
).toString()
113114
}
114115

115-
export function parsePOSTResults(uploadURL: string): {
116-
putURL: string
117-
resultURL: string
118-
} {
116+
117+
118+
export function parsePOSTResults(putAndResultUrlPair: string): PostResults {
119+
info(putAndResultUrlPair)
120+
119121
// JS for [[:graph:]] https://www.regular-expressions.info/posixbrackets.html
120122
const re = /([\x21-\x7E]+)[\r\n]?/gm
121123

122-
const matches = uploadURL.match(re)
124+
const matches = putAndResultUrlPair.match(re)
123125

124126
if (matches === null) {
125-
throw new Error(`Parsing results from POST failed: (${uploadURL})`)
127+
throw new Error(`Parsing results from POST failed: (${putAndResultUrlPair})`)
126128
}
127129

128130
if (matches?.length !== 2) {
@@ -131,8 +133,9 @@ export function parsePOSTResults(uploadURL: string): {
131133
)
132134
}
133135

134-
const putURL = matches[1]
135-
const resultURL = matches[0].trimEnd() // This match may have trailing 0x0A and 0x0D that must be trimmed
136+
const resultURL = new URL(matches[0].trimEnd())
137+
const putURL = new URL(matches[1])
138+
// This match may have trailing 0x0A and 0x0D that must be trimmed
136139

137140
return { putURL, resultURL }
138141
}
@@ -143,7 +146,7 @@ export function displayChangelog(): void {
143146
}
144147

145148
export function generateRequestHeadersPOST(
146-
uploadURL: string,
149+
postURL: URL,
147150
token: string,
148151
query: string,
149152
source: string,
@@ -152,9 +155,9 @@ export function generateRequestHeadersPOST(
152155
if (args.upstream !== '') {
153156
const proxyAgent = new HttpsProxyAgent(args.upstream)
154157
return {
155-
url: `${uploadURL}/upload/v4?package=${getPackage(
158+
url: new URL(`/upload/v4?package=${getPackage(
156159
source,
157-
)}&token=${token}&${query}`,
160+
)}&token=${token}&${query}`, postURL),
158161
options: {
159162
agent: proxyAgent,
160163
method: 'post',
@@ -167,9 +170,9 @@ export function generateRequestHeadersPOST(
167170
}
168171

169172
return {
170-
url: `${uploadURL}/upload/v4?package=${getPackage(
173+
url: new URL(`/upload/v4?package=${getPackage(
171174
source,
172-
)}&token=${token}&${query}`,
175+
)}&token=${token}&${query}`, postURL),
173176
options: {
174177
method: 'post',
175178
headers: {
@@ -181,7 +184,7 @@ export function generateRequestHeadersPOST(
181184
}
182185

183186
export function generateRequestHeadersPUT(
184-
uploadURL: string,
187+
uploadURL: URL,
185188
uploadFile: string | Buffer,
186189
args: UploaderArgs,
187190
): IRequestHeaders {

‎src/index.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,8 @@ export async function main(
298298
const query = webHelpers.generateQuery(buildParams)
299299

300300
if (args.dryRun) {
301-
return dryRun(uploadHost, token, query, uploadFile, args.source || '')
301+
dryRun(uploadHost, token, query, uploadFile, args.source || '')
302+
return
302303
}
303304

304305
info(
@@ -317,28 +318,27 @@ export async function main(
317318
X-Reduced-Redundancy: 'false'`
318319
)
319320

320-
const uploadURL = await webHelpers.uploadToCodecov(
321-
uploadHost,
321+
const postURL = new URL(uploadHost)
322+
323+
const putAndResultUrlPair = await webHelpers.uploadToCodecovPOST(
324+
postURL,
322325
token,
323326
query,
324327
args.source || '',
325328
args,
326329
)
327330

328-
UploadLogger.verbose(`Returned upload url: ${uploadURL}`)
331+
const postResults = webHelpers.parsePOSTResults(putAndResultUrlPair)
329332

330-
UploadLogger.verbose(
331-
`${uploadURL.split('\n')[1]}
332-
Content-Type: 'text/plain'
333-
Content-Encoding: 'gzip'`,
334-
)
335-
const result = await webHelpers.uploadToCodecovPUT(
336-
uploadURL,
333+
UploadLogger.verbose(`Returned upload url: ${postResults.putURL}`)
334+
335+
const statusAndResultPair = await webHelpers.uploadToCodecovPUT(
336+
postResults,
337337
gzippedFile,
338338
args,
339339
)
340-
info(JSON.stringify(result))
341-
return result
340+
info(JSON.stringify(statusAndResultPair))
341+
return {resultURL: statusAndResultPair.resultURL.href, status: statusAndResultPair.status }
342342
} catch (error) {
343343
throw new Error(`Error uploading to ${uploadHost}: ${error}`)
344344
}

‎src/types.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,21 @@ export interface IServiceParams {
6060
}
6161

6262
export interface IRequestHeaders {
63-
url: string;
63+
url: URL;
6464
options: {
6565
method: string,
6666
agent?: Agent
6767
body?: string | Buffer,
6868
headers: Record<string, string>
6969
}
70+
}
71+
72+
export interface PostResults {
73+
putURL: URL
74+
resultURL: URL
75+
}
76+
77+
export interface PutResults {
78+
status: string
79+
resultURL: URL
7080
}

‎test/helpers/web.test.ts‎

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ import {
1010
getPackage,
1111
parsePOSTResults,
1212
populateBuildParams,
13-
uploadToCodecov,
13+
uploadToCodecovPOST,
1414
uploadToCodecovPUT,
1515
} from '../../src/helpers/web'
16-
import { IServiceParams, UploaderArgs } from '../../src/types'
16+
import { IServiceParams, PostResults, UploaderArgs } from '../../src/types'
1717
import { createEmptyArgs } from '../test_helpers'
1818

1919
describe('Web Helpers', () => {
@@ -45,11 +45,17 @@ describe('Web Helpers', () => {
4545
.query(true)
4646
.reply(200, 'testPOSTHTTP')
4747

48-
const response = await uploadToCodecov(uploadURL, token, query, source, {
49-
flags: '',
50-
slug: '',
51-
upstream: '',
52-
})
48+
const response = await uploadToCodecovPOST(
49+
new URL(uploadURL),
50+
token,
51+
query,
52+
source,
53+
{
54+
flags: '',
55+
slug: '',
56+
upstream: '',
57+
},
58+
)
5359
try {
5460
expect(response).toBe('testPOSTHTTP')
5561
} catch (error) {
@@ -66,24 +72,32 @@ describe('Web Helpers', () => {
6672
.query(true)
6773
.reply(200, 'testPOSTHTTPS')
6874

69-
const response = await uploadToCodecov(uploadURL, token, query, source, {
70-
flags: '',
71-
slug: '',
72-
upstream: '',
73-
})
75+
const response = await uploadToCodecovPOST(
76+
new URL(uploadURL),
77+
token,
78+
query,
79+
source,
80+
{
81+
flags: '',
82+
slug: '',
83+
upstream: '',
84+
},
85+
)
7486
expect(response).toBe('testPOSTHTTPS')
7587
})
7688

7789
it('Can PUT to the storage endpoint', async () => {
7890
jest.spyOn(console, 'log').mockImplementation(() => void {})
79-
uploadURL = `https://results.codecov.io
80-
https://codecov.io`
81-
const response = await uploadToCodecovPUT(uploadURL, uploadFile, {
91+
const postResults: PostResults = {
92+
putURL: new URL('https://codecov.io'),
93+
resultURL: new URL('https://results.codecov.io'),
94+
}
95+
const response = await uploadToCodecovPUT(postResults, uploadFile, {
8296
flags: '',
8397
slug: '',
8498
upstream: '',
8599
})
86-
expect(response.resultURL).toEqual('https://results.codecov.io')
100+
expect(response.resultURL.href).toStrictEqual('https://results.codecov.io/')
87101
})
88102

89103
it('Can generate query URL', () => {
@@ -234,17 +248,20 @@ describe('Web Helpers', () => {
234248
})
235249

236250
it('will return an object when parsing correctly and input has multiple linebreaks', () => {
237-
const testURL = `dummyURL
251+
const testURL = `https://result.codecov.local
238252
239253
240254
241255
242256
243257
244258
245-
OtherURL`
246-
const expectedResults = { putURL: 'OtherURL', resultURL: 'dummyURL' }
247-
expect(parsePOSTResults(testURL)).toEqual(expectedResults)
259+
https://put.codecov.local`
260+
const expectedResults = {
261+
putURL: 'https://put.codecov.local/',
262+
resultURL: 'https://result.codecov.local/',
263+
}
264+
expect(JSON.stringify(parsePOSTResults(testURL))).toStrictEqual(JSON.stringify(expectedResults))
248265
})
249266
})
250267
})
@@ -263,15 +280,15 @@ describe('generateRequestHeadersPOST()', () => {
263280
it('should return return the correct url when args.upstream is not set', () => {
264281
args.upstream = ''
265282
const requestHeaders = generateRequestHeadersPOST(
266-
'https:localhost.local',
283+
new URL('https://localhost.local'),
267284
'134',
268285
'slug=testOrg/testUploader',
269286
'G',
270287
args,
271288
)
272289

273-
expect(requestHeaders.url).toEqual(
274-
`https:localhost.local/upload/v4?package=${getPackage(
290+
expect(requestHeaders.url.href).toEqual(
291+
`https://localhost.local/upload/v4?package=${getPackage(
275292
'G',
276293
)}&token=134&slug=testOrg/testUploader`,
277294
)
@@ -282,15 +299,15 @@ describe('generateRequestHeadersPOST()', () => {
282299
it('should return return the correct url when args.upstream is set', () => {
283300
args.upstream = 'http://proxy.local'
284301
const requestHeaders = generateRequestHeadersPOST(
285-
'https:localhost.local',
302+
new URL('https:localhost.local'),
286303
'134',
287304
'slug=testOrg/testUploader',
288305
'G',
289306
args,
290307
)
291308

292-
expect(requestHeaders.url).toEqual(
293-
`https:localhost.local/upload/v4?package=${getPackage(
309+
expect(requestHeaders.url.href).toEqual(
310+
`https://localhost.local/upload/v4?package=${getPackage(
294311
'G',
295312
)}&token=134&slug=testOrg/testUploader`,
296313
)
@@ -308,25 +325,25 @@ describe('generateRequestHeadersPUT()', () => {
308325
it('should return return the correct url when args.upstream is not set', () => {
309326
args.upstream = ''
310327
const requestHeaders = generateRequestHeadersPUT(
311-
'https:localhost.local',
328+
new URL('https://localhost.local'),
312329
"I'm a coverage report!",
313330
args,
314331
)
315332

316-
expect(requestHeaders.url).toEqual('https:localhost.local')
333+
expect(requestHeaders.url.href).toEqual('https://localhost.local/')
317334
expect(requestHeaders.options.body).toEqual("I'm a coverage report!")
318335
expect(typeof requestHeaders.options.agent).toEqual('undefined')
319336
})
320337

321338
it('should return return the correct url when args.upstream is set', () => {
322339
args.upstream = 'http://proxy.local'
323340
const requestHeaders = generateRequestHeadersPUT(
324-
'https:localhost.local',
341+
new URL('https://localhost.local'),
325342
"I'm a coverage report!",
326343
args,
327344
)
328345

329-
expect(requestHeaders.url).toEqual('https:localhost.local')
346+
expect(requestHeaders.url.href).toEqual('https://localhost.local/')
330347
expect(requestHeaders.options.body).toEqual("I'm a coverage report!")
331348
expect(requestHeaders.options.agent).toMatchObject(
332349
new HttpsProxyAgent(args.upstream),

0 commit comments

Comments
 (0)