Skip to content

Commit

Permalink
fix(electron-builder-http): electron-auto-updater request can downloa…
Browse files Browse the repository at this point in the history
…d so fast that the first few chunks arrive before ensureDirPromise has finished for configurePipes to run

Closes #1081
  • Loading branch information
develar committed Jan 4, 2017
1 parent 8d0e230 commit cb85790
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 57 deletions.
2 changes: 1 addition & 1 deletion .idea/rc-producer.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
"test-deps-mac": "brew install rpm dpkg mono lzip gnu-tar graphicsmagick xz && brew install wine --without-x11",
"postinstall": "lerna bootstrap",
"update-deps": "lerna exec -- npm-check-updates --reject 'electron-builder-http'",
"lerna-publish": "lerna publish --skip-npm --skip-git"
"lerna-publish": "lerna publish --skip-npm --skip-git",
"set-versions": "node test/out/helpers/setVersions.js p",
"set-dep-versions": "node test/out/helpers/setVersions.js"
},
"devDependencies": {
"@develar/semantic-release": "^6.3.26",
Expand Down
2 changes: 1 addition & 1 deletion packages/electron-auto-updater/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "electron-auto-updater",
"version": "0.8.5",
"version": "0.0.0-semantic-release",
"description": "NSIS Auto Updater",
"main": "out/main.js",
"author": "Vladimir Krivosheev",
Expand Down
37 changes: 16 additions & 21 deletions packages/electron-auto-updater/src/electronHttpExecutor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { net } from "electron"
import { ensureDir } from "fs-extra-p"
import BluebirdPromise from "bluebird-lst-c"
import * as path from "path"
import { HttpExecutor, DownloadOptions, HttpError, checkSha2, maxRedirects, safeGetHeader, configurePipes } from "electron-builder-http"
import { HttpExecutor, DownloadOptions, HttpError, maxRedirects, safeGetHeader, configurePipes } from "electron-builder-http"
import { safeLoad } from "js-yaml"
import _debug from "debug"
import Debugger = debug.Debugger
Expand All @@ -12,17 +12,21 @@ import { parse as parseUrl } from "url"
export class ElectronHttpExecutor extends HttpExecutor<Electron.RequestOptions, Electron.ClientRequest> {
private readonly debug: Debugger = _debug("electron-builder")

download(url: string, destination: string, options?: DownloadOptions | null): Promise<string> {
return new BluebirdPromise( (resolve, reject) => {
this.doDownload(url, destination, 0, options || {}, (error: Error) => {
if (error == null) {
resolve(destination)
}
else {
reject(error)
}
})
async download(url: string, destination: string, options?: DownloadOptions | null): Promise<string> {
if (options == null || !options.skipDirCreation) {
await ensureDir(path.dirname(destination))
}

return await new BluebirdPromise<string>((resolve, reject) => {
this.doDownload(url, destination, 0, options || {}, (error: Error) => {
if (error == null) {
resolve(destination)
}
else {
reject(error)
}
})
})
}

private addTimeOutHandler(request: Electron.ClientRequest, callback: (error: Error) => void) {
Expand All @@ -35,12 +39,9 @@ export class ElectronHttpExecutor extends HttpExecutor<Electron.RequestOptions,
}

private doDownload(url: string, destination: string, redirectCount: number, options: DownloadOptions, callback: (error: Error | null) => void) {
const ensureDirPromise = options.skipDirCreation ? BluebirdPromise.resolve() : ensureDir(path.dirname(destination))

const parsedUrl = parseUrl(url)
// user-agent must be specified, otherwise some host can return 401 unauthorised

//FIXME hack, the electron typings specifies Protocol with capital but the code actually uses with small case
const requestOpts = {
protocol: parsedUrl.protocol,
hostname: parsedUrl.hostname,
Expand All @@ -67,13 +68,7 @@ export class ElectronHttpExecutor extends HttpExecutor<Electron.RequestOptions,
return
}

if (!checkSha2(safeGetHeader(response, "X-Checksum-Sha2"), options.sha2, callback)) {
return
}

ensureDirPromise
.then(() => configurePipes(options, response, destination, callback))
.catch(callback)
configurePipes(options, response, destination, callback)
})
this.addTimeOutHandler(request, callback)
request.on("error", callback)
Expand Down
6 changes: 5 additions & 1 deletion packages/electron-builder-http/src/httpExecutor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export function request<T>(url: Url, token: string | null = null, data: {[name:
return executorHolder.httpExecutor.request(url, token, data, method, headers)
}

export function checkSha2(sha2Header: string | null | undefined, sha2: string | null | undefined, callback: (error: Error | null) => void): boolean {
function checkSha2(sha2Header: string | null | undefined, sha2: string | null | undefined, callback: (error: Error | null) => void): boolean {
if (sha2Header != null && sha2 != null) {
// todo why bintray doesn't send this header always
if (sha2Header == null) {
Expand Down Expand Up @@ -148,6 +148,10 @@ export function safeGetHeader(response: any, headerKey: string) {
}

export function configurePipes(options: DownloadOptions, response: any, destination: string, callback: (error: Error | null) => void) {
if (!checkSha2(safeGetHeader(response, "X-Checksum-Sha2"), options.sha2, callback)) {
return
}

const streams: Array<any> = []
if (options.onProgress != null) {
const contentLength = safeGetHeader(response, "content-length")
Expand Down
48 changes: 24 additions & 24 deletions packages/electron-builder/src/util/nodeHttpExecutor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,35 @@ import BluebirdPromise from "bluebird-lst-c"
import * as path from "path"
import { homedir } from "os"
import { parse as parseIni } from "ini"
import { HttpExecutor, DownloadOptions, HttpError, configurePipes, checkSha2, maxRedirects } from "electron-builder-http"
import { HttpExecutor, DownloadOptions, HttpError, configurePipes, maxRedirects } from "electron-builder-http"
import { RequestOptions } from "https"
import { safeLoad } from "js-yaml"
import { parse as parseUrl } from "url"
import { debug } from "./util"

export class NodeHttpExecutor extends HttpExecutor<RequestOptions, ClientRequest> {
private httpsAgent: Promise<Agent> | null

download(url: string, destination: string, options?: DownloadOptions | null): Promise<string> {
return <BluebirdPromise<string>>(this.httpsAgent || (this.httpsAgent = createAgent()))
.then(it => new BluebirdPromise((resolve, reject) => {
this.doDownload(url, destination, 0, options || {}, it, (error: Error) => {
if (error == null) {
resolve(destination)
}
else {
reject(error)
}
})
}))
private httpsAgentPromise: Promise<Agent> | null

async download(url: string, destination: string, options?: DownloadOptions | null): Promise<string> {
if (options == null || !options.skipDirCreation) {
await ensureDir(path.dirname(destination))
}

if (this.httpsAgentPromise == null) {
this.httpsAgentPromise = createAgent()
}

const agent = await this.httpsAgentPromise
return await new BluebirdPromise<string>((resolve, reject) => {
this.doDownload(url, destination, 0, options || {}, agent, (error: Error) => {
if (error == null) {
resolve(destination)
}
else {
reject(error)
}
})
})
}

private addTimeOutHandler(request: ClientRequest, callback: (error: Error) => void) {
Expand All @@ -39,8 +47,6 @@ export class NodeHttpExecutor extends HttpExecutor<RequestOptions, ClientRequest
}

private doDownload(url: string, destination: string, redirectCount: number, options: DownloadOptions, agent: Agent, callback: (error: Error | null) => void) {
const ensureDirPromise = options.skipDirCreation ? BluebirdPromise.resolve() : ensureDir(path.dirname(destination))

const parsedUrl = parseUrl(url)
// user-agent must be specified, otherwise some host can return 401 unauthorised
const request = https.request({
Expand All @@ -67,13 +73,7 @@ export class NodeHttpExecutor extends HttpExecutor<RequestOptions, ClientRequest
return
}

if (!checkSha2(response.headers["X-Checksum-Sha2"], options.sha2, callback)) {
return
}

ensureDirPromise
.then(() => configurePipes(options, response, destination, callback))
.catch(callback)
configurePipes(options, response, destination, callback)
})
this.addTimeOutHandler(request, callback)
request.on("error", callback)
Expand Down
37 changes: 32 additions & 5 deletions test/src/helpers/setVersions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,43 @@ import { readdir, readJson, writeJson } from "fs-extra-p"
import BluebirdPromise from "bluebird-lst-c"

const printErrorAndExit = require("../../../packages/electron-builder/out/util/promise").printErrorAndExit
// const exec = require("../../../packages/electron-builder/out/util/util").exec
const exec = require("../../../packages/electron-builder/out/util/util").exec

const rootDir = path.join(__dirname, "../../..")
const packageDir = path.join(rootDir, "packages")

async function main(): Promise<void> {
const rootDir = path.join(__dirname, "../../..")
const packageDir = path.join(rootDir, "packages")
const packages = (await readdir(packageDir)).filter(it => !it.includes(".")).sort()
// const versions = await BluebirdPromise.map(packages, it => exec("node", [path.join(rootDir, "test", "vendor", "yarn.js"), "info", "--json", it, "dist-tags"]).then((it: string) => JSON.parse(it).data))

const packageData = await BluebirdPromise.map(packages, it => readJson(path.join(packageDir, it, "package.json")))
const versions = packageData.map(it => it.version)
const args = process.argv.slice(2)
if (args.length > 0 && args[0] === "p") {
await setPackageVersions(packages, packageData)
}
else {
await setDepVersions(packages, packageData)
}
}

async function setPackageVersions(packages: Array<string>, packageData: Array<any>) {
const versions = await BluebirdPromise.map(packages, it => exec("node", [path.join(rootDir, "test", "vendor", "yarn.js"), "info", "--json", it, "dist-tags"]).then((it: string) => JSON.parse(it).data))
for (let i = 0; i < packages.length; i++) {
const packageName = packages[i]
const packageJson = packageData[i]
const versionInfo = versions[i]
const latestVersion = versionInfo.next || versionInfo.latest
if (packageJson.version == latestVersion) {
continue
}

packageJson.version = latestVersion
console.log(`Set ${packageName} version to ${latestVersion}`)
await writeJson(path.join(packageDir, packageName, "package.json"), packageJson, {spaces: 2})
}
}

async function setDepVersions(packages: Array<string>, packageData: Array<any>) {
const versions = packageData.map(it => it.version)
for (let version of versions) {
if (version == "0.0.0-semantic-release") {
throw new Error(`Semantic version 0.0.0-semantic-release is detected, please fix it`)
Expand Down
10 changes: 7 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1749,8 +1749,8 @@ istanbul-lib-hook@^1.0.0-alpha.4:
append-transform "^0.3.0"

istanbul-lib-instrument@^1.1.1, istanbul-lib-instrument@^1.3.0, istanbul-lib-instrument@^1.4.0:
version "1.4.0"
resolved "https://registry.yarnpkg.com/istanbul-lib-instrument/-/istanbul-lib-instrument-1.4.0.tgz#33da8df669da74e532ba409e0397db0dc56be093"
version "1.4.1"
resolved "https://registry.yarnpkg.com/istanbul-lib-instrument/-/istanbul-lib-instrument-1.4.1.tgz#9b98c18e327198d24c0bbbf9091fb988aff2d976"
dependencies:
babel-generator "^6.18.0"
babel-template "^6.16.0"
Expand Down Expand Up @@ -2396,14 +2396,18 @@ mime@^1.2.11:
dependencies:
brace-expansion "^1.0.0"

minimist@0.0.8, minimist@~0.0.1:
minimist@0.0.8:
version "0.0.8"
resolved "https://registry.yarnpkg.com/minimist/-/minimist-0.0.8.tgz#857fcabfc3397d2625b8228262e86aa7a011b05d"

minimist@^1.1.0, minimist@^1.1.1, minimist@^1.1.3, minimist@^1.2.0:
version "1.2.0"
resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.0.tgz#a35008b20f41383eec1fb914f4cd5df79a264284"

minimist@~0.0.1:
version "0.0.10"
resolved "https://registry.yarnpkg.com/minimist/-/minimist-0.0.10.tgz#de3f98543dbf96082be48ad1a0c7cda836301dcf"

mkdirp@^0.5.0, mkdirp@^0.5.1, mkdirp@~0.5.0:
version "0.5.1"
resolved "https://registry.yarnpkg.com/mkdirp/-/mkdirp-0.5.1.tgz#30057438eac6cf7f8c4767f38648d6697d75c903"
Expand Down

0 comments on commit cb85790

Please sign in to comment.