From b1a3f20932af5ed0359073061df6388e7fddfb17 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Fri, 10 Jan 2020 18:12:28 -0500 Subject: [PATCH 1/3] code review --- .github/workflows/test.yml | 2 +- LICENSE | 2 +- __tests__/auth.test.ts | 5 ++-- __tests__/basics.test.ts | 5 +--- __tests__/keepalive.test.ts | 8 +------ __tests__/proxy.test.ts | 2 +- auth.ts | 4 ++-- index.ts | 6 ++--- package-lock.json | 47 +------------------------------------ package.json | 4 +--- proxy.ts | 4 ++-- tsconfig.json | 5 +--- 12 files changed, 16 insertions(+), 78 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f68538e..1c265e2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,7 +7,7 @@ on: - '**.md' pull_request: paths-ignore: - - '**.md' + - '**.md' jobs: diff --git a/LICENSE b/LICENSE index d1ba4ff..5823a51 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Typed Rest Client for Node.js +Actions Http Client for Node.js Copyright (c) GitHub, Inc. diff --git a/__tests__/auth.test.ts b/__tests__/auth.test.ts index a194e70..b981c8b 100644 --- a/__tests__/auth.test.ts +++ b/__tests__/auth.test.ts @@ -1,6 +1,5 @@ -import * as httpm from '../_out'; -import * as path from 'path'; -import * as am from '../_out/auth'; +import * as httpm from '../'; +import * as am from '../auth'; describe('auth', () => { beforeEach(() => { diff --git a/__tests__/basics.test.ts b/__tests__/basics.test.ts index 0706162..ed4a4a0 100644 --- a/__tests__/basics.test.ts +++ b/__tests__/basics.test.ts @@ -1,14 +1,11 @@ -import * as httpm from '../_out'; +import * as httpm from '../'; import * as path from 'path'; -import * as am from '../_out/auth'; import * as fs from 'fs'; -import { connect } from 'http2'; let sampleFilePath: string = path.join(__dirname, 'testoutput.txt'); describe('basics', () => { let _http: httpm.HttpClient; - let _httpbin: httpm.HttpClient; beforeEach(() => { _http = new httpm.HttpClient('http-client-tests'); diff --git a/__tests__/keepalive.test.ts b/__tests__/keepalive.test.ts index 1afe802..c7183fe 100644 --- a/__tests__/keepalive.test.ts +++ b/__tests__/keepalive.test.ts @@ -1,13 +1,7 @@ -import * as httpm from '../_out'; -import * as path from 'path'; -import * as am from '../_out/auth'; -import * as fs from 'fs'; - -let sampleFilePath: string = path.join(__dirname, 'testoutput.txt'); +import * as httpm from '../'; describe('basics', () => { let _http: httpm.HttpClient; - let _httpbin: httpm.HttpClient; beforeEach(() => { _http = new httpm.HttpClient('http-client-tests', [], { keepAlive: true }); diff --git a/__tests__/proxy.test.ts b/__tests__/proxy.test.ts index 45ef0b0..e0d81d0 100644 --- a/__tests__/proxy.test.ts +++ b/__tests__/proxy.test.ts @@ -1,4 +1,4 @@ -import * as pm from '../_out/proxy'; +import * as pm from '../proxy'; import * as url from 'url'; describe('proxy', () => { diff --git a/auth.ts b/auth.ts index 2b6d168..ae00211 100644 --- a/auth.ts +++ b/auth.ts @@ -11,7 +11,7 @@ export class BasicCredentialHandler implements ifm.IRequestHandler { } prepareRequest(options:any): void { - options.headers['Authorization'] = 'Basic ' + new Buffer(this.username + ':' + this.password).toString('base64'); + options.headers['Authorization'] = 'Basic ' + Buffer.from(this.username + ':' + this.password).toString('base64'); } // This handler cannot handle 401 @@ -57,7 +57,7 @@ export class PersonalAccessTokenCredentialHandler implements ifm.IRequestHandler // currently implements pre-authorization // TODO: support preAuth = false where it hooks on 401 prepareRequest(options:any): void { - options.headers['Authorization'] = 'Basic ' + new Buffer('PAT:' + this.token).toString('base64'); + options.headers['Authorization'] = 'Basic ' + Buffer.from('PAT:' + this.token).toString('base64'); } // This handler cannot handle 401 diff --git a/index.ts b/index.ts index 69324bd..25efee5 100644 --- a/index.ts +++ b/index.ts @@ -4,7 +4,6 @@ import https = require("https"); import ifm = require('./interfaces'); import pm = require('./proxy'); -let fs: any; let tunnel: any; export enum HttpCodes { @@ -70,7 +69,7 @@ export function isHttps(requestUrl: string) { } export class HttpClient { - userAgent: string | null | undefined; + userAgent: string | undefined; handlers: ifm.IRequestHandler[]; requestOptions: ifm.IRequestOptions; @@ -276,8 +275,7 @@ export class HttpClient { */ public requestRawWithCallback(info: ifm.IRequestInfo, data: string | NodeJS.ReadableStream, onResult: (err: any, res: ifm.IHttpClientResponse) => void): void { let socket; - - let isDataString = typeof (data) === 'string'; + if (typeof (data) === 'string') { info.options.headers["Content-Length"] = Buffer.byteLength(data, 'utf8'); } diff --git a/package-lock.json b/package-lock.json index e883513..1061632 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@actions/http-client", - "version": "1.0.0", + "version": "1.0.1", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -2381,12 +2381,6 @@ "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==", "dev": true }, - "interpret": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/interpret/-/interpret-1.2.0.tgz", - "integrity": "sha512-mT34yGKMNceBQUoVn7iCDKDntA7SC6gycMAWzGx1z/CMCTV7b2AAtXlo3nRyHZ1FelRkQbQjprHSYGwzLtkVbw==", - "dev": true - }, "invariant": { "version": "2.2.4", "resolved": "https://registry.npmjs.org/invariant/-/invariant-2.2.4.tgz", @@ -3473,19 +3467,6 @@ "integrity": "sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ==", "dev": true }, - "nock": { - "version": "11.7.2", - "resolved": "https://registry.npmjs.org/nock/-/nock-11.7.2.tgz", - "integrity": "sha512-7swr5bL1xBZ5FctyubjxEVySXOSebyqcL7Vy1bx1nS9IUqQWj81cmKjVKJLr8fHhtzI1MV8nyCdENA/cGcY1+Q==", - "dev": true, - "requires": { - "debug": "^4.1.0", - "json-stringify-safe": "^5.0.1", - "lodash": "^4.17.13", - "mkdirp": "^0.5.0", - "propagate": "^2.0.0" - } - }, "node-int64": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/node-int64/-/node-int64-0.4.0.tgz", @@ -3839,12 +3820,6 @@ "sisteransi": "^1.0.3" } }, - "propagate": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/propagate/-/propagate-2.0.1.tgz", - "integrity": "sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==", - "dev": true - }, "psl": { "version": "1.7.0", "resolved": "https://registry.npmjs.org/psl/-/psl-1.7.0.tgz", @@ -3909,15 +3884,6 @@ "util.promisify": "^1.0.0" } }, - "rechoir": { - "version": "0.6.2", - "resolved": "https://registry.npmjs.org/rechoir/-/rechoir-0.6.2.tgz", - "integrity": "sha1-hSBLVNuoLVdC4oyWdW70OvUOM4Q=", - "dev": true, - "requires": { - "resolve": "^1.1.6" - } - }, "regex-not": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/regex-not/-/regex-not-1.0.2.tgz", @@ -4177,17 +4143,6 @@ "integrity": "sha1-2kL0l0DAtC2yypcoVxyxkMmO/qM=", "dev": true }, - "shelljs": { - "version": "0.8.3", - "resolved": "https://registry.npmjs.org/shelljs/-/shelljs-0.8.3.tgz", - "integrity": "sha512-fc0BKlAWiLpwZljmOvAOTE/gXawtCoNrP5oaY7KIaQbbyHeQVg01pSEuEGvGh3HEdBU4baCD7wQBwADmM/7f7A==", - "dev": true, - "requires": { - "glob": "^7.0.0", - "interpret": "^1.0.0", - "rechoir": "^0.6.2" - } - }, "shellwords": { "version": "0.1.1", "resolved": "https://registry.npmjs.org/shellwords/-/shellwords-0.1.1.tgz", diff --git a/package.json b/package.json index e08023a..07c4170 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "description": "Actions Http Client", "main": "index.js", "scripts": { - "build": "rm -Rf ./_out && tsc && cp package*.json ./_out && cp *.md ./_out && cp LICENSE ./_out && cp actions.png ./_out", + "build": "tsc", "test": "jest" }, "repository": { @@ -26,8 +26,6 @@ "@types/node": "^13.1.5", "@types/shelljs": "^0.8.6", "jest": "^24.9.0", - "nock": "^11.7.2", - "shelljs": "^0.8.3", "ts-jest": "^24.3.0", "typescript": "^3.7.4" } diff --git a/proxy.ts b/proxy.ts index 2d171f2..c251575 100644 --- a/proxy.ts +++ b/proxy.ts @@ -17,7 +17,7 @@ export function getProxyUrl(reqUrl: url.Url): url.Url { bypass = true; break; } - } + } } let proxyUrl: url.Url; @@ -34,7 +34,7 @@ export function getProxyUrl(reqUrl: url.Url): url.Url { proxyVar = process.env["http_proxy"] || process.env["HTTP_PROXY"]; } - + if (proxyVar) { proxyUrl = url.parse(proxyVar); } diff --git a/tsconfig.json b/tsconfig.json index 560cca0..9047cdc 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -8,8 +8,5 @@ "outDir": "_out", "forceConsistentCasingInFileNames": true }, - "files": [ - "index.ts", - "auth.ts" - ] + "include": ["*.ts"] } \ No newline at end of file From 62b5d3a94f0f3440a320534f483b4d6fd0ecc963 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Fri, 10 Jan 2020 18:17:31 -0500 Subject: [PATCH 2/3] Remove @types/shelljs --- package-lock.json | 33 --------------------------------- package.json | 1 - 2 files changed, 34 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1061632..ffe44e7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -401,23 +401,6 @@ "@babel/types": "^7.3.0" } }, - "@types/events": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/@types/events/-/events-3.0.0.tgz", - "integrity": "sha512-EaObqwIvayI5a8dCzhFrjKzVwKLxjoG9T6Ppd5CEo07LRKfQ8Yokw54r5+Wq7FaBQ+yXRvQAYPrHwya1/UFt9g==", - "dev": true - }, - "@types/glob": { - "version": "7.1.1", - "resolved": "https://registry.npmjs.org/@types/glob/-/glob-7.1.1.tgz", - "integrity": "sha512-1Bh06cbWJUHMC97acuD6UMG29nMt0Aqz1vF3guLfG+kHHJhy3AyohZFFxYk2f7Q1SQIrNwvncxAE0N/9s70F2w==", - "dev": true, - "requires": { - "@types/events": "*", - "@types/minimatch": "*", - "@types/node": "*" - } - }, "@types/istanbul-lib-coverage": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.1.tgz", @@ -452,28 +435,12 @@ "jest-diff": "^24.3.0" } }, - "@types/minimatch": { - "version": "3.0.3", - "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-3.0.3.tgz", - "integrity": "sha512-tHq6qdbT9U1IRSGf14CL0pUlULksvY9OZ+5eEgl1N7t+OA3tGvNpxJCzuKQlsNgCVwbAs670L1vcVQi8j9HjnA==", - "dev": true - }, "@types/node": { "version": "13.1.5", "resolved": "https://registry.npmjs.org/@types/node/-/node-13.1.5.tgz", "integrity": "sha512-wupvfmtbqRJzjCm1H2diy7wo31Gn1OzvqoxCfQuKM9eSecogzP0WTlrjdq7cf7jgSO2ZX6hxwgRPR8Wt7FA22g==", "dev": true }, - "@types/shelljs": { - "version": "0.8.6", - "resolved": "https://registry.npmjs.org/@types/shelljs/-/shelljs-0.8.6.tgz", - "integrity": "sha512-svx2eQS268awlppL/P8wgDLBrsDXdKznABHJcuqXyWpSKJgE1s2clXlBvAwbO/lehTmG06NtEWJRkAk4tAgenA==", - "dev": true, - "requires": { - "@types/glob": "*", - "@types/node": "*" - } - }, "@types/stack-utils": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/@types/stack-utils/-/stack-utils-1.0.1.tgz", diff --git a/package.json b/package.json index 07c4170..652f095 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,6 @@ "devDependencies": { "@types/jest": "^24.0.25", "@types/node": "^13.1.5", - "@types/shelljs": "^0.8.6", "jest": "^24.9.0", "ts-jest": "^24.3.0", "typescript": "^3.7.4" From 7f15359c45b81c7ba5e360de449ed0bf878839de Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Sat, 11 Jan 2020 21:23:55 -0500 Subject: [PATCH 3/3] Revert unnecessary changes --- __tests__/auth.test.ts | 4 ++-- __tests__/basics.test.ts | 2 +- __tests__/keepalive.test.ts | 2 +- __tests__/proxy.test.ts | 2 +- package-lock.json | 19 +++++++++++++++++++ package.json | 3 ++- tsconfig.json | 5 ++++- 7 files changed, 30 insertions(+), 7 deletions(-) diff --git a/__tests__/auth.test.ts b/__tests__/auth.test.ts index b981c8b..ac2ab52 100644 --- a/__tests__/auth.test.ts +++ b/__tests__/auth.test.ts @@ -1,5 +1,5 @@ -import * as httpm from '../'; -import * as am from '../auth'; +import * as httpm from '../_out'; +import * as am from '../_out/auth'; describe('auth', () => { beforeEach(() => { diff --git a/__tests__/basics.test.ts b/__tests__/basics.test.ts index ed4a4a0..b67b3f7 100644 --- a/__tests__/basics.test.ts +++ b/__tests__/basics.test.ts @@ -1,4 +1,4 @@ -import * as httpm from '../'; +import * as httpm from '../_out'; import * as path from 'path'; import * as fs from 'fs'; diff --git a/__tests__/keepalive.test.ts b/__tests__/keepalive.test.ts index c7183fe..7d14774 100644 --- a/__tests__/keepalive.test.ts +++ b/__tests__/keepalive.test.ts @@ -1,4 +1,4 @@ -import * as httpm from '../'; +import * as httpm from '../_out'; describe('basics', () => { let _http: httpm.HttpClient; diff --git a/__tests__/proxy.test.ts b/__tests__/proxy.test.ts index e0d81d0..45ef0b0 100644 --- a/__tests__/proxy.test.ts +++ b/__tests__/proxy.test.ts @@ -1,4 +1,4 @@ -import * as pm from '../proxy'; +import * as pm from '../_out/proxy'; import * as url from 'url'; describe('proxy', () => { diff --git a/package-lock.json b/package-lock.json index ffe44e7..aa62741 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3434,6 +3434,19 @@ "integrity": "sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ==", "dev": true }, + "nock": { + "version": "11.7.2", + "resolved": "https://registry.npmjs.org/nock/-/nock-11.7.2.tgz", + "integrity": "sha512-7swr5bL1xBZ5FctyubjxEVySXOSebyqcL7Vy1bx1nS9IUqQWj81cmKjVKJLr8fHhtzI1MV8nyCdENA/cGcY1+Q==", + "dev": true, + "requires": { + "debug": "^4.1.0", + "json-stringify-safe": "^5.0.1", + "lodash": "^4.17.13", + "mkdirp": "^0.5.0", + "propagate": "^2.0.0" + } + }, "node-int64": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/node-int64/-/node-int64-0.4.0.tgz", @@ -3787,6 +3800,12 @@ "sisteransi": "^1.0.3" } }, + "propagate": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/propagate/-/propagate-2.0.1.tgz", + "integrity": "sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==", + "dev": true + }, "psl": { "version": "1.7.0", "resolved": "https://registry.npmjs.org/psl/-/psl-1.7.0.tgz", diff --git a/package.json b/package.json index 652f095..5bc9155 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "description": "Actions Http Client", "main": "index.js", "scripts": { - "build": "tsc", + "build": "rm -Rf ./_out && tsc && cp package*.json ./_out && cp *.md ./_out && cp LICENSE ./_out && cp actions.png ./_out", "test": "jest" }, "repository": { @@ -25,6 +25,7 @@ "@types/jest": "^24.0.25", "@types/node": "^13.1.5", "jest": "^24.9.0", + "nock": "^11.7.2", "ts-jest": "^24.3.0", "typescript": "^3.7.4" } diff --git a/tsconfig.json b/tsconfig.json index 9047cdc..560cca0 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -8,5 +8,8 @@ "outDir": "_out", "forceConsistentCasingInFileNames": true }, - "include": ["*.ts"] + "files": [ + "index.ts", + "auth.ts" + ] } \ No newline at end of file