Skip to content

Commit

Permalink
PB-977: Don't use service-proxy for internal domain and for GPX that …
Browse files Browse the repository at this point in the history
…support CORS

GPX file used always the service-proxy even if the server supported CORS. Also
on the service-proxy we could see that some requests were made over service-proxy
for internal server like public.geo.admin.ch. This was due to the fact that if
for any reason the initial request failed (network failure) we fallback to the
service proxy. Now in that case we don't fallback anymore.

NOTE unfortunately there is no way for a web application to check for CORS support
see https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS/Errors
  • Loading branch information
ltshb committed Sep 10, 2024
1 parent c0334e4 commit 555957d
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 81 deletions.
38 changes: 0 additions & 38 deletions src/api/file-proxy.api.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import axios from 'axios'
import { isString } from 'lodash'

import { getServiceProxyBaseUrl } from '@/config/baseUrl.config'
Expand Down Expand Up @@ -57,40 +56,3 @@ export function proxifyUrl(url) {
}
return `${getServiceProxyBaseUrl()}${fileAsPath}`
}

/**
* Get a file through our service-proxy backend, taking care of CORS headers in the process.
*
* That means that a file for which there is no defined CORS header will still be accessible through
* this function (i.e. Dropbox/name your cloud share links)
*
* @param {String} fileUrl
* @param {Object} [options]
* @param {Number} [options.timeout] How long should the call wait before timing out
* @returns {Promise<AxiosResponse>} A promise which resolve to the proxy response
*/
export default function getFileThroughProxy(fileUrl, options = {}) {
const { timeout = null } = options
return new Promise((resolve, reject) => {
try {
axios({
method: 'get',
url: proxifyUrl(fileUrl),
timeout,
})
.then((response) => {
resolve(response)
})
.catch((error) => {
log.error(
'Error while accessing file URL through service-proxy',
fileUrl,
error
)
reject(error)
})
} catch (error) {
reject(error)
}
})
}
69 changes: 52 additions & 17 deletions src/api/files.api.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import axios, { AxiosError } from 'axios'
import axios from 'axios'
import FormData from 'form-data'
import pako from 'pako'

import getFileThroughProxy from '@/api/file-proxy.api'
import { proxifyUrl } from '@/api/file-proxy.api'
import { getServiceKmlBaseUrl } from '@/config/baseUrl.config'
import log from '@/utils/logging'
import { isInternalUrl } from '@/utils/utils'

/**
* KML links
Expand Down Expand Up @@ -312,8 +313,7 @@ export function loadKmlData(kmlLayer) {
new Error(`No file URL defined in this KML layer, cannot load data ${kmlLayer.id}`)
)
}
axios
.get(kmlLayer.kmlFileUrl)
getFileFromUrl(kmlLayer.kmlFileUrl)
.then((response) => {
if (response.status === 200 && response.data) {
resolve(response.data)
Expand All @@ -324,19 +324,54 @@ export function loadKmlData(kmlLayer) {
}
})
.catch((error) => {
if (error instanceof AxiosError) {
getFileThroughProxy(kmlLayer.kmlFileUrl)
.then((response) => {
resolve(response.data)
})
.catch((error) => {
reject(error)
})
} else {
const msg = `Failed to load KML data: ${error}`
log.error(msg)
reject(new Error(msg))
}
const msg = `Failed to load KML data: ${error}`
log.error(msg)
reject(new Error(msg))
})
})
}

/**
* Generic function to load a file from a given URL.
*
* When the URL is not an internal url and it doesn't support CORS or use HTTP it is sent over a
* proxy.
*
* @param {string} url URL to fetch
* @param {Number} [options.timeout] How long should the call wait before timing out
* @returns {Promise<AxiosResponse<any, any>>}
*/
export async function getFileFromUrl(url, options = {}) {
const { timeout = null } = options
if (/^https?:\/\/localhost/.test(url) || isInternalUrl(url)) {
// don't go through proxy if it is on localhost or the internal server
return axios.get(url, { timeout })
} else if (url.startsWith('http://')) {
// HTTP request goes through the proxy
return axios.get(proxifyUrl(url), { timeout })
}

// For other urls we need to check if they support CORS
let supportCORS = false
try {
// unfortunately we cannot do a real preflight call using options because browser don't
// allow to set the Access-Control-* headers ! Also there is no way to find out if a request
// is failing due to network reason or due to CORS issue,
// see https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS/Errors
// Therefore here we try to get the resource using head instead
await axios.head(url, { timeout })
supportCORS = true
} catch (error) {
log.error(
`URL ${url} failed with ${error}. It might be due to CORS issue, ` +
`therefore the request will be fallback to the service-proxy`
)
}

if (supportCORS) {
// Server support CORS
return axios.get(url, { timeout })
}
// server don't support CORS use proxy
return axios.get(proxifyUrl(url), { timeout })
}
3 changes: 3 additions & 0 deletions src/config/baseUrl.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,3 +206,6 @@ export function get3dTilesBaseUrl() {
export function getVectorTilesBaseUrl() {
return getBaseUrl('vectorTiles')
}

export const internalDomainRegex =
import.meta.env.VITE_APP_INTERNAL_DOMAIN_REGEX ?? /^https:\/\/[^/]*(bgdi|admin)\.ch/
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<script setup>
import axios, { AxiosError } from 'axios'
import { AxiosError } from 'axios'
import { computed, onMounted, ref, toRefs, watch } from 'vue'
import { useStore } from 'vuex'
import getFileThroughProxy from '@/api/file-proxy.api'
import { getFileFromUrl } from '@/api/files.api'
import ImportFileButtons from '@/modules/menu/components/advancedTools/ImportFile/ImportFileButtons.vue'
import { handleFileContent } from '@/modules/menu/components/advancedTools/ImportFile/utils'
import TextInput from '@/utils/components/TextInput.vue'
Expand Down Expand Up @@ -79,20 +79,9 @@ async function loadFile() {
return
}
loading.value = true
let response
try {
// catching locally the first error, so that we may decide to use service-proxy if the error was network related
try {
response = await axios.get(fileUrl.value, { timeout: REQUEST_TIMEOUT })
} catch (error) {
if (error instanceof AxiosError) {
log.debug('Failed to retrieve file directly, trying to go through service-proxy')
response = await getFileThroughProxy(fileUrl.value, { timeout: REQUEST_TIMEOUT })
} else {
throw error
}
}
const response = await getFileFromUrl(fileUrl.value, { timeout: REQUEST_TIMEOUT })
if (response.status !== 200) {
throw new Error(`Failed to fetch ${fileUrl.value}; status_code=${response.status}`)
}
Expand Down
5 changes: 2 additions & 3 deletions src/store/plugins/load-gpx-data.plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
* it here
*/

import axios from 'axios'
import GPX from 'ol/format/GPX'

import { proxifyUrl } from '@/api/file-proxy.api'
import { getFileFromUrl } from '@/api/files.api'
import GPXLayer from '@/api/layers/GPXLayer.class'
import log from '@/utils/logging'

Expand All @@ -20,7 +19,7 @@ const dispatcher = { dispatcher: 'load-gpx-data.plugin' }
async function loadGpx(store, gpxLayer) {
log.debug(`Loading data for added GPX layer`, gpxLayer)
try {
const response = await axios.get(proxifyUrl(gpxLayer.gpxFileUrl))
const response = await getFileFromUrl(gpxLayer.gpxFileUrl)
const gpxContent = response.data
const gpxParser = new GPX()
const metadata = gpxParser.readMetadata(gpxContent)
Expand Down
2 changes: 1 addition & 1 deletion src/utils/kmlUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ export function getEditableFeatureFromKmlFeature(kmlFeature, kmlLayer, available
const nonGeoadminIconUrls = new Set()
export function iconUrlProxyFy(url, corsIssueCallback = null) {
// We only proxyfy URL that are not from our backend.
if (!/^(https:\/\/[^/]*(bgdi\.ch|geo\.admin\.ch)|http:\/\/localhost)/.test(url)) {
if (!/^(https:\/\/[^/]*(bgdi\.ch|geo\.admin\.ch)|https?:\/\/localhost)/.test(url)) {
const proxyUrl = proxifyUrl(url)
// Only perform the CORS check if we have a callback and it has not yet been done
if (!nonGeoadminIconUrls.has(url) && corsIssueCallback) {
Expand Down
11 changes: 11 additions & 0 deletions src/utils/utils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { LineString, Point, Polygon } from 'ol/geom'

import { internalDomainRegex } from '@/config/baseUrl.config'
import { toLv95 } from '@/utils/coordinates/coordinateUtils'
import log from '@/utils/logging'
import { format } from '@/utils/numberUtils'
Expand Down Expand Up @@ -249,3 +250,13 @@ export function humanFileSize(size) {
const i = size == 0 ? 0 : Math.floor(Math.log(size) / Math.log(1024))
return (size / Math.pow(1024, i)).toFixed(2) * 1 + ' ' + ['B', 'kB', 'MB', 'GB', 'TB'][i]
}

/**
* Check if the given url is an internal URL (from bgdi.ch or admin.ch subdomain)
*
* @param {string} url
* @returns {boolean} Returns true if the url is part of an internal server
*/
export function isInternalUrl(url) {
return internalDomainRegex.test(url)
}
70 changes: 62 additions & 8 deletions tests/cypress/tests-e2e/importToolFile.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ describe('The Import File Tool', () => {
// Test the import of an online KML file
cy.log('Test online import')
const localKmlFile = 'import-tool/external-kml-file.kml'
const validOnlineUrl = 'http://example.com/valid-kml-file.kml'
const validOnlineUrl = 'https://example.com/valid-kml-file.kml'
cy.intercept('HEAD', validOnlineUrl, {
statusCode: 200,
}).as('headKmlFile')
cy.intercept('GET', validOnlineUrl, {
fixture: localKmlFile,
}).as('getKmlFile')
Expand Down Expand Up @@ -57,7 +60,10 @@ describe('The Import File Tool', () => {
//----------------------------------------------------------------------
cy.log('Test adding another external online KML layer')
const secondLocalKmlFile = 'import-tool/second-external-kml-file.kml'
const secondValidOnlineUrl = 'http://example.com/second-valid-kml-file.kml'
const secondValidOnlineUrl = 'https://example.com/second-valid-kml-file.kml'
cy.intercept('HEAD', secondValidOnlineUrl, {
statusCode: 200,
}).as('headSecondKmlFile')
cy.intercept('GET', secondValidOnlineUrl, {
fixture: secondLocalKmlFile,
}).as('getSecondKmlFile')
Expand Down Expand Up @@ -231,22 +237,47 @@ describe('The Import File Tool', () => {
cy.get('[data-cy="menu-section-active-layers"]:visible').children().should('have.length', 1)
cy.get(`[data-cy^="active-layer-name-${secondValidOnlineUrl}-"]`).should('be.visible')
cy.get('[data-cy^="button-loading-metadata-spinner-"]').should('not.exist')

// Test the import of an online KML file that don't support CORS
cy.log('Test online import - Non CORS server')
const validOnlineNonCORSUrl = 'https://example.com/valid-kml-file-non-cors.kml'
cy.intercept('HEAD', validOnlineNonCORSUrl, { forceNetworkError: true }).as(
'headKmlCorsFile'
)
cy.intercept('GET', proxifyUrl(validOnlineNonCORSUrl), {
fixture: localKmlFile,
}).as('getKmlCorsFile')

cy.openMenuIfMobile()
cy.get('[data-cy="menu-tray-tool-section"]:visible').click()
cy.get('[data-cy="menu-advanced-tools-import-file"]:visible').click()

// Type a valid online GPX file URL
cy.get('[data-cy="text-input"]:visible').type(validOnlineNonCORSUrl)
cy.get('[data-cy="import-file-load-button"]:visible').click()
cy.wait('@headKmlCorsFile')
cy.wait('@getKmlCorsFile')
cy.readStoreValue('state.layers.activeLayers').should('have.length', 2)
})
it('Import KML file error handling', () => {
const outOfBoundKMLFile = 'import-tool/paris.kml'
const emptyKMLFile = 'import-tool/empty.kml'

const invalidFileOnlineUrl = 'http://example.com/invalid-file.kml'
cy.intercept('HEAD', 'https://example.com/*', {
statusCode: 200,
})

const invalidFileOnlineUrl = 'https://example.com/invalid-file.kml'
cy.intercept('GET', invalidFileOnlineUrl, {
body: `<html>Not a KML</html>`,
}).as('getInvalidKmlFile')

const onlineUrlNotReachable = 'http://example.com/kml-file.kml'
const onlineUrlNotReachable = 'https://example.com/kml-file.kml'
cy.intercept('GET', onlineUrlNotReachable, {
statusCode: 403,
}).as('getNoReachableKmlFile')

const outOfBoundKMLUrl = 'http://example.com/out-of-bound-kml-file.kml'
const outOfBoundKMLUrl = 'https://example.com/out-of-bound-kml-file.kml'
cy.intercept('GET', outOfBoundKMLUrl, {
fixture: outOfBoundKMLFile,
}).as('getOutOfBoundKmlFile')
Expand Down Expand Up @@ -387,7 +418,7 @@ describe('The Import File Tool', () => {
//----------------------------------------------------------------------
// Attach an online empty KML file
cy.log('Test add an online empty KML file')
const emptyKMLUrl = 'http://example.com/empty-kml-file.kml'
const emptyKMLUrl = 'https://example.com/empty-kml-file.kml'
cy.intercept('GET', emptyKMLUrl, {
fixture: emptyKMLFile,
}).as('getEmptyKmlFile')
Expand Down Expand Up @@ -501,9 +532,10 @@ describe('The Import File Tool', () => {

// Test the import of an online GPX file
cy.log('Test online import')
const validOnlineUrl = 'http://example.com/valid-gpx-file.gpx'
const validOnlineUrl = 'https://example.com/valid-gpx-file.gpx'
const gpxOnlineLayerId = `GPX|${validOnlineUrl}`
cy.intercept('GET', proxifyUrl(validOnlineUrl), {
cy.intercept('HEAD', validOnlineUrl, { statusCode: 200 })
cy.intercept('GET', validOnlineUrl, {
fixture: gpxFileFixture,
}).as('getGpxFile')

Expand Down Expand Up @@ -570,5 +602,27 @@ describe('The Import File Tool', () => {
cy.openMenuIfMobile()
cy.get(`[data-cy^="button-remove-layer-${gpxOnlineLayerId}-"]:visible`).click()
cy.readStoreValue('state.layers.activeLayers').should('be.empty')

// Test the import of an online GPX file that don't support CORS
cy.log('Test online import - Non CORS server')
const validOnlineNonCORSUrl = 'https://example.com/valid-gpx-file-non-cors.gpx'
const gpxOnlineLayerNonCORSId = `GPX|${validOnlineNonCORSUrl}`
cy.intercept('HEAD', validOnlineNonCORSUrl, { forceNetworkError: true }).as(
'headGpxCorsFile'
)
cy.intercept('GET', proxifyUrl(validOnlineNonCORSUrl), {
fixture: gpxFileFixture,
}).as('getGpxCorsFile')

cy.openMenuIfMobile()
cy.get('[data-cy="menu-tray-tool-section"]:visible').click()
cy.get('[data-cy="menu-advanced-tools-import-file"]:visible').click()

// Type a valid online GPX file URL
cy.get('[data-cy="text-input"]:visible').type(validOnlineNonCORSUrl)
cy.get('[data-cy="import-file-load-button"]:visible').click()
cy.wait('@headGpxCorsFile')
cy.wait('@getGpxCorsFile')
cy.checkOlLayer([bgLayer, gpxOnlineLayerNonCORSId])
})
})

0 comments on commit 555957d

Please sign in to comment.