From a88d30140cba438563deb729c77a4cce4bfd7d83 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Mon, 13 Jan 2020 15:39:13 -0800
Subject: [PATCH] fix(geolocation): improve geolocation validation (#471)

---
 src/browserContext.ts    | 28 +++++++++++++++++-----------
 src/types.ts             |  6 +++---
 test/geolocation.spec.js | 19 +++++++++++++++++++
 3 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/src/browserContext.ts b/src/browserContext.ts
index 17f1cbac91..8db3953f19 100644
--- a/src/browserContext.ts
+++ b/src/browserContext.ts
@@ -18,6 +18,7 @@
 import { Page } from './page';
 import * as network from './network';
 import * as types from './types';
+import { helper } from './helper';
 
 export interface BrowserContextDelegate {
   pages(): Promise<Page[]>;
@@ -58,7 +59,7 @@ export class BrowserContext {
     if (this._options.viewport)
       this._options.viewport = { ...this._options.viewport };
     if (this._options.geolocation)
-      this._options.geolocation = { ...this._options.geolocation };
+      this._options.geolocation = verifyGeolocation(this._options.geolocation);
   }
 
   async _initialize() {
@@ -100,16 +101,8 @@ export class BrowserContext {
   }
 
   async setGeolocation(geolocation: types.Geolocation | null): Promise<void> {
-    if (geolocation) {
-      geolocation.accuracy = geolocation.accuracy || 0;
-      const { longitude, latitude, accuracy } = geolocation;
-      if (longitude !== undefined && (longitude < -180 || longitude > 180))
-        throw new Error(`Invalid longitude "${longitude}": precondition -180 <= LONGITUDE <= 180 failed.`);
-      if (latitude !== undefined && (latitude < -90 || latitude > 90))
-        throw new Error(`Invalid latitude "${latitude}": precondition -90 <= LATITUDE <= 90 failed.`);
-      if (accuracy < 0)
-        throw new Error(`Invalid accuracy "${accuracy}": precondition 0 <= ACCURACY failed.`);
-    }
+    if (geolocation)
+      geolocation = verifyGeolocation(geolocation);
     this._options.geolocation = geolocation || undefined;
     await this._delegate.setGeolocation(geolocation);
   }
@@ -121,3 +114,16 @@ export class BrowserContext {
     this._closed = true;
   }
 }
+
+function verifyGeolocation(geolocation: types.Geolocation): types.Geolocation {
+  const result = { ...geolocation };
+  result.accuracy = result.accuracy || 0;
+  const { longitude, latitude, accuracy } = result;
+  if (!helper.isNumber(longitude) || longitude < -180 || longitude > 180)
+    throw new Error(`Invalid longitude "${longitude}": precondition -180 <= LONGITUDE <= 180 failed.`);
+  if (!helper.isNumber(latitude) || latitude < -90 || latitude > 90)
+    throw new Error(`Invalid latitude "${latitude}": precondition -90 <= LATITUDE <= 90 failed.`);
+  if (!helper.isNumber(accuracy) || accuracy < 0)
+    throw new Error(`Invalid accuracy "${accuracy}": precondition 0 <= ACCURACY failed.`);
+  return result;
+}
diff --git a/src/types.ts b/src/types.ts
index f45f10b0b1..7bc9a9f53d 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -68,9 +68,9 @@ export type Credentials = {
 };
 
 export type Geolocation = {
-  longitude?: number;
-  latitude?: number;
-  accuracy?: number | undefined;
+  longitude: number;
+  latitude: number;
+  accuracy?: number;
 };
 
 export type SelectOption = {
diff --git a/test/geolocation.spec.js b/test/geolocation.spec.js
index 327aa63e79..5af6dbe521 100644
--- a/test/geolocation.spec.js
+++ b/test/geolocation.spec.js
@@ -42,6 +42,15 @@ module.exports.describe = function ({ testRunner, expect, FFOX, WEBKIT }) {
       }
       expect(error.message).toContain('Invalid longitude "200"');
     });
+    it('should throw with missing latitude', async({context}) => {
+      let error = null;
+      try {
+        await context.setGeolocation({longitude: 10});
+      } catch (e) {
+        error = e;
+      }
+      expect(error.message).toContain('Invalid latitude "undefined"');
+    });
     it('should not modify passed default options object', async({newContext}) => {
       const geolocation = { longitude: 10, latitude: 10 };
       const options = { geolocation };
@@ -49,6 +58,16 @@ module.exports.describe = function ({ testRunner, expect, FFOX, WEBKIT }) {
       await context.setGeolocation({ longitude: 20, latitude: 20 });
       expect(options.geolocation).toBe(geolocation);
     });
+    it('should throw with missing longitude in default options', async({newContext}) => {
+      let error = null;
+      try {
+        const context = await newContext({ geolocation: {latitude: 10} });
+        await context.close();
+      } catch (e) {
+        error = e;
+      }
+      expect(error.message).toContain('Invalid longitude "undefined"');
+    });
     it('should use context options', async({newContext, server}) => {
       const options = { geolocation: { longitude: 10, latitude: 10 }, permissions: {} };
       options.permissions[server.PREFIX] = ['geolocation'];