fix(electron-auto-updater): Checking for updates from github was failing

Closes #1038
electron-userland · Dec 31, 2016 · 3401630 · 3401630
1 parent 43add64
commit 3401630
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 90 deletions.
diff --git a/packages/electron-auto-updater/README.md b/packages/electron-auto-updater/README.md
@@ -1 +1,3 @@
+# electron-auto-updater
+
 [Auto Update](https://github.com/electron-userland/electron-builder/wiki/Auto-Update).
diff --git a/packages/electron-auto-updater/src/GitHubProvider.ts b/packages/electron-auto-updater/src/GitHubProvider.ts
@@ -11,18 +11,27 @@ export class GitHubProvider implements Provider<VersionInfo> {
   async getLatestVersion(): Promise<UpdateInfo> {
     // do not use API to avoid limit
     const basePath = this.getBasePath()
-    let version = (await request<Redirect>({hostname: "github.com", path: `${basePath}/latest`})).location
-    const versionPosition = version.lastIndexOf("/") + 1
+    let version = (await request<GithubReleaseInfo>(
+      {hostname: "github.com", path: `${basePath}/latest`},
+      null,
+      null,
+      "GET",
+      {"Accept": "application/json"}
+    )).tag_name
+
     try {
-      version = version.substring(version[versionPosition] === "v" ? versionPosition + 1 : versionPosition)
+      version = (version.startsWith("v")) ? version.substring(1) : version
     }
     catch (e) {
-      throw new Error(`Cannot parse extract version from location "${version}": ${e.stack || e.message}`)
+      throw new Error(`Cannot parse determine latest version from github "${version}": ${e.stack || e.message}`)
     }
 
     let result: UpdateInfo | null = null
     try {
-      result = await request<UpdateInfo>({hostname: "github.com", path: `https://github.com${basePath}/download/v${version}/latest.yml`})
+      result = await request<UpdateInfo>({
+        hostname: "github.com",
+        path: `https://github.com${basePath}/download/v${version}/latest.yml`
+      })
     }
     catch (e) {
       if (e instanceof HttpError && e.response.statusCode === 404) {
@@ -51,6 +60,6 @@ export class GitHubProvider implements Provider<VersionInfo> {
   }
 }
 
-interface Redirect {
-  readonly location: string
+interface GithubReleaseInfo {
+  readonly tag_name: string
 }
diff --git a/packages/electron-auto-updater/src/electronHttpExecutor.ts b/packages/electron-auto-updater/src/electronHttpExecutor.ts
@@ -3,8 +3,7 @@ import { net } from "electron"
 import { createWriteStream, ensureDir } from "fs-extra-p"
 import BluebirdPromise from "bluebird-lst-c"
 import * as path from "path"
-import { HttpExecutor, DownloadOptions, HttpError, DigestTransform, checkSha2, calculateDownloadProgress } from "electron-builder-http/out/httpExecutor"
-import { Url } from "url"
+import { HttpExecutor, DownloadOptions, HttpError, DigestTransform, checkSha2, calculateDownloadProgress, maxRedirects } from "electron-builder-http/out/httpExecutor"
 import { safeLoad } from "js-yaml"
 import _debug from "debug"
 import Debugger = debug.Debugger
@@ -14,32 +13,9 @@ function safeGetHeader(response: Electron.IncomingMessage, headerKey: string) {
   return response.headers[headerKey] ? response.headers[headerKey].pop() : null
 }
 
-export class ElectronHttpExecutor implements HttpExecutor {
+export class ElectronHttpExecutor extends HttpExecutor<Electron.RequestOptions, Electron.ClientRequest> {
   private readonly debug: Debugger = _debug("electron-builder")
 
-  private readonly maxRedirects = 10
-
-  request<T>(url: Url, token: string | null = null, data: {[name: string]: any; } | null = null, method: string = "GET"): Promise<T> {
-    const options: any = Object.assign({
-      method: method,
-      headers: {
-        "User-Agent": "electron-builder"
-      }
-    }, url)
-
-    if (url.hostname!!.includes("github") && !url.path!.endsWith(".yml")) {
-      options.headers.Accept = "application/vnd.github.v3+json"
-    }
-
-    const encodedData = data == null ? undefined : new Buffer(JSON.stringify(data))
-    if (encodedData != null) {
-      options.method = "post"
-      options.headers["Content-Type"] = "application/json"
-      options.headers["Content-Length"] = encodedData.length
-    }
-    return this.doApiRequest<T>(options, token, it => it.end(encodedData))
-  }
-
   download(url: string, destination: string, options?: DownloadOptions | null): Promise<string> {
       return new BluebirdPromise( (resolve, reject) => {
         this.doDownload(url, destination, 0, options || {}, (error: Error) => {
@@ -86,11 +62,11 @@ export class ElectronHttpExecutor implements HttpExecutor {
 
       const redirectUrl = safeGetHeader(response, "location")
       if (redirectUrl != null) {
-        if (redirectCount < this.maxRedirects) {
+        if (redirectCount < maxRedirects) {
           this.doDownload(redirectUrl, destination, redirectCount++, options, callback)
         }
         else {
-          callback(new Error("Too many redirects (> " + this.maxRedirects + ")"))
+          callback(new Error(`Too many redirects (> ${maxRedirects})`))
         }
         return
       }
@@ -162,14 +138,10 @@ Please double check that your authentication token is correct. Due to security r
               return
             }
 
-            if (options.path!.endsWith("/latest")) {
-              resolve(<any>{location: redirectUrl})
-            }
-            else {
-              this.doApiRequest(Object.assign({}, options, parseUrl(redirectUrl)), token, requestProcessor)
-                .then(<any>resolve)
-                .catch(reject)
-            }
+            this.doApiRequest(Object.assign({}, options, parseUrl(redirectUrl)), token, requestProcessor)
+              .then(<any>resolve)
+              .catch(reject)
+
             return
           }
 
@@ -182,7 +154,8 @@ Please double check that your authentication token is correct. Due to security r
           response.on("end", () => {
             try {
               const contentType = response.headers["content-type"]
-              const isJson = contentType != null && contentType.includes("json")
+              const isJson = contentType != null && contentType.find((i) => i.indexOf("json") !== -1)
+              const isYaml = options.path!.includes(".yml")
               if (response.statusCode >= 400) {
                 if (isJson) {
                   reject(new HttpError(response, JSON.parse(data)))
@@ -192,7 +165,7 @@ Please double check that your authentication token is correct. Due to security r
                 }
               }
               else {
-                resolve(data.length === 0 ? null : (isJson || !options.path!.includes(".yml")) ? JSON.parse(data) : safeLoad(data))
+                resolve(data.length === 0 ? null : (isJson ? JSON.parse(data) : isYaml ? safeLoad(data) : data))
               }
             }
             catch (e) {

diff --git a/packages/electron-builder-http/readme.md b/packages/electron-builder-http/readme.md
@@ -0,0 +1,3 @@
+# electron-builder-http
+
+Part of [electron-builder](https://github.com/electron-userland/electron-builder).
diff --git a/packages/electron-builder-http/src/httpExecutor.ts b/packages/electron-builder-http/src/httpExecutor.ts
@@ -9,24 +9,47 @@ export interface DownloadOptions {
 }
 
 export class HttpExecutorHolder {
-  private _httpExecutor: HttpExecutor
+  private _httpExecutor: HttpExecutor<any, any>
 
-  get httpExecutor(): HttpExecutor {
+  get httpExecutor(): HttpExecutor<any, any> {
     if (this._httpExecutor == null) {
       this._httpExecutor = new (require("electron-builder/out/util/nodeHttpExecutor").NodeHttpExecutor)()
     }
     return this._httpExecutor
   }
 
-  set httpExecutor(value: HttpExecutor) {
+  set httpExecutor(value: HttpExecutor<any, any>) {
     this._httpExecutor = value
   }
 }
 
-export interface HttpExecutor {
-  request<T>(url: Url, token?: string | null, data?: {[name: string]: any; } | null, method?: string): Promise<T>
+export const maxRedirects = 10
 
-  download(url: string, destination: string, options?: DownloadOptions | null): Promise<string>
+export abstract class HttpExecutor<REQUEST_OPTS, REQUEST> {
+  request<T>(url: Url, token?: string | null, data?: {[name: string]: any; } | null, method?: string, headers?: any): Promise<T> {
+    const defaultHeaders = {"User-Agent": "electron-builder"}
+    const options = Object.assign({
+      method: method,
+      headers: headers == null ? defaultHeaders : Object.assign(defaultHeaders, headers)
+    }, url)
+
+
+    if (url.hostname!!.includes("github") && !url.path!.endsWith(".yml") && !options.headers.Accept) {
+      options.headers["Accept"] = "application/vnd.github.v3+json"
+    }
+
+    const encodedData = data == null ? undefined : new Buffer(JSON.stringify(data))
+    if (encodedData != null) {
+      options.method = "post"
+      options.headers["Content-Type"] = "application/json"
+      options.headers["Content-Length"] = encodedData.length
+    }
+    return this.doApiRequest<T>(<any>options, token || null, it => (<any>it).end(encodedData), 0)
+  }
+
+  protected abstract doApiRequest<T>(options: REQUEST_OPTS, token: string | null, requestProcessor: (request: REQUEST, reject: (error: Error) => void) => void, redirectCount: number): Promise<T>
+
+  abstract download(url: string, destination: string, options?: DownloadOptions | null): Promise<string>
 }
 
 export class HttpError extends Error {
@@ -59,8 +82,8 @@ export function githubRequest<T>(path: string, token: string | null, data: {[nam
   return request<T>({hostname: "api.github.com", path: path}, token, data, method)
 }
 
-export function request<T>(url: Url, token: string | null = null, data: {[name: string]: any; } | null = null, method: string = "GET"): Promise<T> {
-  return executorHolder.httpExecutor.request(url, token, data, method)
+export function request<T>(url: Url, token: string | null = null, data: {[name: string]: any; } | null = null, method: string = "GET", headers?: any): Promise<T> {
+  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 {

diff --git a/packages/electron-builder/src/util/nodeHttpExecutor.ts b/packages/electron-builder/src/util/nodeHttpExecutor.ts
@@ -6,39 +6,15 @@ 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, DigestTransform, checkSha2, calculateDownloadProgress} from "electron-builder-http/out/httpExecutor"
-import { Url } from "url"
+import { HttpExecutor, DownloadOptions, HttpError, DigestTransform, checkSha2, calculateDownloadProgress, maxRedirects } from "electron-builder-http/out/httpExecutor"
 import { RequestOptions } from "https"
 import { safeLoad } from "js-yaml"
 import { parse as parseUrl } from "url"
 import { debug } from "./util"
 
-export class NodeHttpExecutor implements HttpExecutor {
-  private readonly maxRedirects = 10
-
+export class NodeHttpExecutor extends HttpExecutor<RequestOptions, ClientRequest> {
   private httpsAgent: Promise<Agent> | null = null
 
-  request<T>(url: Url, token: string | null = null, data: {[name: string]: any; } | null = null, method: string = "GET"): Promise<T> {
-    const options: any = Object.assign({
-      method: method,
-      headers: {
-        "User-Agent": "electron-builder"
-      }
-    }, url)
-
-    if (url.hostname!!.includes("github") && !url.path!.endsWith(".yml")) {
-      options.headers.Accept = "application/vnd.github.v3+json"
-    }
-
-    const encodedData = data == null ? null : new Buffer(JSON.stringify(data))
-    if (encodedData != null) {
-      options.method = "post"
-      options.headers["Content-Type"] = "application/json"
-      options.headers["Content-Length"] = encodedData.length
-    }
-    return this.doApiRequest<T>(options, token, it => it.end(encodedData))
-  }
-
   download(url: string, destination: string, options?: DownloadOptions | null): Promise<string> {
     return <BluebirdPromise<string>>(this.httpsAgent || (this.httpsAgent = createAgent()))
       .then(it => new BluebirdPromise( (resolve, reject) => {
@@ -82,11 +58,11 @@ export class NodeHttpExecutor implements HttpExecutor {
 
       const redirectUrl = response.headers.location
       if (redirectUrl != null) {
-        if (redirectCount < this.maxRedirects) {
+        if (redirectCount < maxRedirects) {
           this.doDownload(redirectUrl, destination, redirectCount++, options, agent, callback)
         }
         else {
-          callback(new Error("Too many redirects (> " + this.maxRedirects + ")"))
+          callback(new Error(`Too many redirects (> ${maxRedirects})`))
         }
         return
       }
@@ -159,14 +135,10 @@ Please double check that your authentication token is correct. Due to security r
               return
             }
 
-            if (options.path!.endsWith("/latest")) {
-              resolve(<any>{location: redirectUrl})
-            }
-            else {
-              this.doApiRequest(Object.assign({}, options, parseUrl(redirectUrl)), token, requestProcessor)
-                .then(<any>resolve)
-                .catch(reject)
-            }
+            this.doApiRequest(Object.assign({}, options, parseUrl(redirectUrl)), token, requestProcessor)
+              .then(<any>resolve)
+              .catch(reject)
+
             return
           }
 
@@ -180,6 +152,7 @@ Please double check that your authentication token is correct. Due to security r
             try {
               const contentType = response.headers["content-type"]
               const isJson = contentType != null && contentType.includes("json")
+              const isYaml = options.path!.includes(".yml")
               if (response.statusCode >= 400) {
                 if (isJson) {
                   reject(new HttpError(response, JSON.parse(data)))
@@ -189,7 +162,7 @@ Please double check that your authentication token is correct. Due to security r
                 }
               }
               else {
-                resolve(data.length === 0 ? null : (isJson || !options.path!.includes(".yml")) ? JSON.parse(data) : safeLoad(data))
+                resolve(data.length === 0 ? null : (isJson ? JSON.parse(data) : isYaml ? safeLoad(data) : data))
               }
             }
             catch (e) {
Original file line number	Diff line number	Diff line change
		@@ -1 +1,3 @@
		# electron-auto-updater

		[Auto Update](https://github.com/electron-userland/electron-builder/wiki/Auto-Update).
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		# electron-builder-http

		Part of [electron-builder](https://github.com/electron-userland/electron-builder).