Skip to content

Commit

Permalink
Merge pull request #1061 from geoadmin/bug-PB-977-proxy
Browse files Browse the repository at this point in the history
PB-977: Don't use service-proxy for internal domain and for GPX that support CORS - #patch
  • Loading branch information
ltshb authored Sep 11, 2024
2 parents 7b10584 + 2dcf44e commit 87535b9
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 87535b9

Please sign in to comment.