Repository: codecov/uploader
Pull request #604 (2022-02-01 17:12:47)

refactor: use url objects for input sanitization


Patch:
diff --git a/src/helpers/web.ts b/src/helpers/web.ts
index 516d1ce..4481bbb 100644
--- a/src/helpers/web.ts
+++ b/src/helpers/web.ts
@@ -5,6 +5,8 @@ import { version } from '../../package.json'
 import {
   IRequestHeaders,
   IServiceParams,
+  PostResults,
+  PutResults,
   UploaderArgs,
   UploaderInputs,
 } from '../types'
@@ -49,17 +51,16 @@ export function getPackage(source: string): string {
   }
 }
 
+
 export async function uploadToCodecovPUT(
-  uploadURL: string,
+  putAndResultUrlPair: PostResults, 
   uploadFile: string | Buffer,
   args: UploaderArgs
-): Promise<{ status: string; resultURL: string }> {
+): Promise<PutResults> {
   info('Uploading...')
 
-  const { putURL, resultURL } = parsePOSTResults(uploadURL)
-
   const requestHeaders = generateRequestHeadersPUT(
-    putURL,
+    putAndResultUrlPair.putURL,
     uploadFile,
     args,
   )
@@ -72,18 +73,18 @@ export async function uploadToCodecovPUT(
     )
   }
 
-  return { status: 'success', resultURL }
+  return { status: 'success', resultURL: putAndResultUrlPair.resultURL }
 }
 
-export async function uploadToCodecov(
-  uploadURL: string,
+export async function uploadToCodecovPOST(
+  postURL: URL,
   token: string,
   query: string,
   source: string,
   args: UploaderArgs,
 ): Promise<string> {
   const requestHeaders = generateRequestHeadersPOST(
-    uploadURL,
+    postURL,
     token,
     query,
     source,
@@ -112,17 +113,18 @@ export function generateQuery(queryParams: Partial<IServiceParams>): string {
   ).toString()
 }
 
-export function parsePOSTResults(uploadURL: string): {
-  putURL: string
-  resultURL: string
-} {
+
+
+export function parsePOSTResults(putAndResultUrlPair: string): PostResults {
+  info(putAndResultUrlPair)
+
   // JS for [[:graph:]] https://www.regular-expressions.info/posixbrackets.html
   const re = /([\x21-\x7E]+)[\r\n]?/gm
 
-  const matches = uploadURL.match(re)
+  const matches = putAndResultUrlPair.match(re)
 
   if (matches === null) {
-    throw new Error(`Parsing results from POST failed: (${uploadURL})`)
+    throw new Error(`Parsing results from POST failed: (${putAndResultUrlPair})`)
   }
 
   if (matches?.length !== 2) {
@@ -131,8 +133,9 @@ export function parsePOSTResults(uploadURL: string): {
     )
   }
 
-  const putURL = matches[1]
-  const resultURL = matches[0].trimEnd() // This match may have trailing 0x0A and 0x0D that must be trimmed
+  const resultURL = new URL(matches[0].trimEnd())
+  const putURL = new URL(matches[1])
+   // This match may have trailing 0x0A and 0x0D that must be trimmed
 
   return { putURL, resultURL }
 }
@@ -143,7 +146,7 @@ export function displayChangelog(): void {
 }
 
 export function generateRequestHeadersPOST(
-  uploadURL: string,
+  postURL: URL,
   token: string,
   query: string,
   source: string,
@@ -152,9 +155,9 @@ export function generateRequestHeadersPOST(
     if (args.upstream !== '') {
       const proxyAgent = new HttpsProxyAgent(args.upstream)
       return {
-        url: `${uploadURL}/upload/v4?package=${getPackage(
+        url: new URL(`/upload/v4?package=${getPackage(
           source,
-        )}&token=${token}&${query}`,
+        )}&token=${token}&${query}`, postURL),
         options: {
           agent: proxyAgent,
           method: 'post',
@@ -167,9 +170,9 @@ export function generateRequestHeadersPOST(
     }
 
     return {
-      url: `${uploadURL}/upload/v4?package=${getPackage(
+      url: new URL(`/upload/v4?package=${getPackage(
         source,
-      )}&token=${token}&${query}`,
+      )}&token=${token}&${query}`, postURL),
       options: {
         method: 'post',
         headers: {
@@ -181,7 +184,7 @@ export function generateRequestHeadersPOST(
 }
 
 export function generateRequestHeadersPUT(
-  uploadURL: string,
+  uploadURL: URL,
   uploadFile: string | Buffer,
   args: UploaderArgs,
 ): IRequestHeaders {
diff --git a/src/index.ts b/src/index.ts
index 0170cc7..8161198 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -298,7 +298,8 @@ export async function main(
   const query = webHelpers.generateQuery(buildParams)
 
   if (args.dryRun) {
-    return dryRun(uploadHost, token, query, uploadFile, args.source || '')
+    dryRun(uploadHost, token, query, uploadFile, args.source || '')
+    return
   }
 
   info(
@@ -317,28 +318,27 @@ export async function main(
         X-Reduced-Redundancy: 'false'`
     )
 
-    const uploadURL = await webHelpers.uploadToCodecov(
-      uploadHost,
+    const postURL = new URL(uploadHost)
+
+    const putAndResultUrlPair = await webHelpers.uploadToCodecovPOST(
+      postURL,
       token,
       query,
       args.source || '',
       args,
     )
 
-    UploadLogger.verbose(`Returned upload url: ${uploadURL}`)
+    const postResults = webHelpers.parsePOSTResults(putAndResultUrlPair)
 
-    UploadLogger.verbose(
-      `${uploadURL.split('\n')[1]}
-        Content-Type: 'text/plain'
-        Content-Encoding: 'gzip'`,
-    )
-    const result = await webHelpers.uploadToCodecovPUT(
-      uploadURL,
+    UploadLogger.verbose(`Returned upload url: ${postResults.putURL}`)
+
+    const statusAndResultPair = await webHelpers.uploadToCodecovPUT(
+      postResults,
       gzippedFile,
       args,
     )
-    info(JSON.stringify(result))
-    return result
+    info(JSON.stringify(statusAndResultPair))
+    return {resultURL: statusAndResultPair.resultURL.href, status: statusAndResultPair.status }
   } catch (error) {
     throw new Error(`Error uploading to ${uploadHost}: ${error}`)
   }
diff --git a/src/types.ts b/src/types.ts
index 3ab6938..6d86e07 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -60,11 +60,21 @@ export interface IServiceParams {
 }
 
 export interface IRequestHeaders {
-  url: string;
+  url: URL;
   options: {
     method: string,
     agent?: Agent
     body?: string | Buffer,
     headers: Record<string, string>
   } 
+}
+
+export interface PostResults {
+  putURL: URL
+  resultURL: URL
+}
+
+export interface PutResults {
+  status: string
+  resultURL: URL
 }
\ No newline at end of file


Test patch:
diff --git a/test/helpers/web.test.ts b/test/helpers/web.test.ts
index 515e7ca..8336329 100644
--- a/test/helpers/web.test.ts
+++ b/test/helpers/web.test.ts
@@ -10,10 +10,10 @@ import {
   getPackage,
   parsePOSTResults,
   populateBuildParams,
-  uploadToCodecov,
+  uploadToCodecovPOST,
   uploadToCodecovPUT,
 } from '../../src/helpers/web'
-import { IServiceParams, UploaderArgs } from '../../src/types'
+import { IServiceParams, PostResults, UploaderArgs } from '../../src/types'
 import { createEmptyArgs } from '../test_helpers'
 
 describe('Web Helpers', () => {
@@ -45,11 +45,17 @@ describe('Web Helpers', () => {
       .query(true)
       .reply(200, 'testPOSTHTTP')
 
-    const response = await uploadToCodecov(uploadURL, token, query, source, {
-      flags: '',
-      slug: '',
-      upstream: '',
-    })
+    const response = await uploadToCodecovPOST(
+      new URL(uploadURL),
+      token,
+      query,
+      source,
+      {
+        flags: '',
+        slug: '',
+        upstream: '',
+      },
+    )
     try {
       expect(response).toBe('testPOSTHTTP')
     } catch (error) {
@@ -66,24 +72,32 @@ describe('Web Helpers', () => {
       .query(true)
       .reply(200, 'testPOSTHTTPS')
 
-    const response = await uploadToCodecov(uploadURL, token, query, source, {
-      flags: '',
-      slug: '',
-      upstream: '',
-    })
+    const response = await uploadToCodecovPOST(
+      new URL(uploadURL),
+      token,
+      query,
+      source,
+      {
+        flags: '',
+        slug: '',
+        upstream: '',
+      },
+    )
     expect(response).toBe('testPOSTHTTPS')
   })
 
   it('Can PUT to the storage endpoint', async () => {
     jest.spyOn(console, 'log').mockImplementation(() => void {})
-    uploadURL = `https://results.codecov.io
-    https://codecov.io`
-    const response = await uploadToCodecovPUT(uploadURL, uploadFile, {
+    const postResults: PostResults = {
+      putURL: new URL('https://codecov.io'),
+      resultURL: new URL('https://results.codecov.io'),
+    }
+    const response = await uploadToCodecovPUT(postResults, uploadFile, {
       flags: '',
       slug: '',
       upstream: '',
     })
-    expect(response.resultURL).toEqual('https://results.codecov.io')
+    expect(response.resultURL.href).toStrictEqual('https://results.codecov.io/')
   })
 
   it('Can generate query URL', () => {
@@ -234,7 +248,7 @@ describe('Web Helpers', () => {
     })
 
     it('will return an object when parsing correctly and input has multiple linebreaks', () => {
-      const testURL = `dummyURL
+      const testURL = `https://result.codecov.local
       
       
       
@@ -242,9 +256,12 @@ describe('Web Helpers', () => {
       
       
       
-      OtherURL`
-      const expectedResults = { putURL: 'OtherURL', resultURL: 'dummyURL' }
-      expect(parsePOSTResults(testURL)).toEqual(expectedResults)
+      https://put.codecov.local`
+      const expectedResults = {
+        putURL: 'https://put.codecov.local/',
+        resultURL: 'https://result.codecov.local/',
+      }
+      expect(JSON.stringify(parsePOSTResults(testURL))).toStrictEqual(JSON.stringify(expectedResults))
     })
   })
 })
@@ -263,15 +280,15 @@ describe('generateRequestHeadersPOST()', () => {
   it('should return return the correct url when args.upstream is not set', () => {
     args.upstream = ''
     const requestHeaders = generateRequestHeadersPOST(
-      'https:localhost.local',
+      new URL('https://localhost.local'),
       '134',
       'slug=testOrg/testUploader',
       'G',
       args,
     )
 
-    expect(requestHeaders.url).toEqual(
-      `https:localhost.local/upload/v4?package=${getPackage(
+    expect(requestHeaders.url.href).toEqual(
+      `https://localhost.local/upload/v4?package=${getPackage(
         'G',
       )}&token=134&slug=testOrg/testUploader`,
     )
@@ -282,15 +299,15 @@ describe('generateRequestHeadersPOST()', () => {
   it('should return return the correct url when args.upstream is set', () => {
     args.upstream = 'http://proxy.local'
     const requestHeaders = generateRequestHeadersPOST(
-      'https:localhost.local',
+      new URL('https:localhost.local'),
       '134',
       'slug=testOrg/testUploader',
       'G',
       args,
     )
 
-    expect(requestHeaders.url).toEqual(
-      `https:localhost.local/upload/v4?package=${getPackage(
+    expect(requestHeaders.url.href).toEqual(
+      `https://localhost.local/upload/v4?package=${getPackage(
         'G',
       )}&token=134&slug=testOrg/testUploader`,
     )
@@ -308,12 +325,12 @@ describe('generateRequestHeadersPUT()', () => {
   it('should return return the correct url when args.upstream is not set', () => {
     args.upstream = ''
     const requestHeaders = generateRequestHeadersPUT(
-      'https:localhost.local',
+      new URL('https://localhost.local'),
       "I'm a coverage report!",
       args,
     )
 
-    expect(requestHeaders.url).toEqual('https:localhost.local')
+    expect(requestHeaders.url.href).toEqual('https://localhost.local/')
     expect(requestHeaders.options.body).toEqual("I'm a coverage report!")
     expect(typeof requestHeaders.options.agent).toEqual('undefined')
   })
@@ -321,12 +338,12 @@ describe('generateRequestHeadersPUT()', () => {
   it('should return return the correct url when args.upstream is set', () => {
     args.upstream = 'http://proxy.local'
     const requestHeaders = generateRequestHeadersPUT(
-      'https:localhost.local',
+      new URL('https://localhost.local'),
       "I'm a coverage report!",
       args,
     )
 
-    expect(requestHeaders.url).toEqual('https:localhost.local')
+    expect(requestHeaders.url.href).toEqual('https://localhost.local/')
     expect(requestHeaders.options.body).toEqual("I'm a coverage report!")
     expect(requestHeaders.options.agent).toMatchObject(
       new HttpsProxyAgent(args.upstream),
diff --git a/test/index.test.ts b/test/index.test.ts
index ea90285..182d3aa 100644
--- a/test/index.test.ts
+++ b/test/index.test.ts
@@ -72,7 +72,7 @@ describe('Uploader Core', () => {
     })
     expect(result).toEqual({
       status: 'success',
-      resultURL: 'https://results.codecov.io',
+      resultURL: 'https://results.codecov.io/',
     })
   }, 30000)
 
@@ -151,7 +151,7 @@ describe('Uploader Core', () => {
       })
       expect(result).toEqual({
         status: 'success',
-        resultURL: 'https://results.codecov.io',
+        resultURL: 'https://results.codecov.io/',
       })
     }, 30000)
   })
@@ -179,7 +179,7 @@ describe('Uploader Core', () => {
     })
     expect(result).toEqual({
       status: 'success',
-      resultURL: 'https://results.codecov.io',
+      resultURL: 'https://results.codecov.io/',
     })
   }, 30000)