Skip to content

Commit

Permalink
FSR-1208 | Fix 2 (#759)
Browse files Browse the repository at this point in the history
* handle non-latin characters

* add allowed char. add else if conditional

* undo else

* move a conditional to the handler

* move a conditional to the handler

---------

Co-authored-by: Ash <[email protected]>
  • Loading branch information
ashleyjtaylor and Ash committed Jul 18, 2024
1 parent a15ba9c commit 44d1efc
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 14 deletions.
27 changes: 20 additions & 7 deletions server/routes/alerts-and-warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ const {
failActionHandler,
renderNotFound,
renderLocationNotFound,
createQueryParametersString
createQueryParametersString,
hasInvalidCharacters
} = require('./lib/utils')

const route = 'alerts-and-warnings'
Expand All @@ -37,17 +38,17 @@ async function routeHandler (request, h) {
return h.view(route, { model })
}

if (hasInvalidCharacters(location, request.query.q)) {
return renderNotFound(location)
}

if (!location) {
const data = await request.server.methods.flood.getFloods()
floods = new Floods(data)
model = new ViewModel({ location, floods })
return h.view(route, { model })
}

if (isLocationEngland(location)) {
return h.redirect(`/${route}`)
}

const [place] = await locationService.find(location)

if (!place) {
Expand Down Expand Up @@ -106,7 +107,13 @@ async function locationRouteHandler (request, h) {
module.exports = [{
method: 'GET',
path: `/${route}`,
handler: routeHandler,
handler: (request, h) => {
if (request.query.q && isLocationEngland(util.cleanseLocation(request.query.q))) {
return h.redirect(`/${route}`)
}

return routeHandler(request, h)
},
options: {
validate: {
query: joi.object({
Expand Down Expand Up @@ -146,7 +153,13 @@ module.exports = [{
{
method: 'POST',
path: `/${route}`,
handler: routeHandler,
handler: (request, h) => {
if (isLocationEngland(util.cleanseLocation(request.payload.location))) {
return h.redirect(`/${route}`)
}

return routeHandler(request, h)
},
options: {
validate: {
payload: joi.object({
Expand Down
5 changes: 5 additions & 0 deletions server/routes/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ function slugify (text = '') {
return text.replace(/,/g, '').replace(/ /g, '-').toLowerCase()
}

function hasInvalidCharacters (location, q) {
return !location && q
}

function filterDisambiguationPlaces (places) {
return isPlaceEngland(places[0]) ? places : []
}
Expand Down Expand Up @@ -63,6 +67,7 @@ module.exports = {
isLocationEngland,
isPlaceEngland,
isValidLocationSlug,
hasInvalidCharacters,
renderNotFound,
renderLocationNotFound,
createQueryParametersString,
Expand Down
23 changes: 17 additions & 6 deletions server/routes/river-and-sea-levels.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ const {
filterDisambiguationPlaces,
isValidLocationSlug,
isLocationEngland,
isPlaceEngland
isPlaceEngland,
hasInvalidCharacters
} = require('./lib/utils')

const route = 'river-and-sea-levels'
Expand Down Expand Up @@ -66,12 +67,12 @@ async function locationQueryHandler (request, h) {

request.yar.set('q', { location })

if (!location) {
return h.view(route, { model: emptyResultsModel() })
if (hasInvalidCharacters(location, request.query.q)) {
return renderNotFound(location)
}

if (isLocationEngland(location)) {
return h.redirect(`/${route}`)
if (!location) {
return h.view(route, { model: emptyResultsModel() })
}

const rivers = await request.server.methods.flood.getRiversByName(location)
Expand Down Expand Up @@ -213,6 +214,10 @@ module.exports = [{
// note: the redirects below are to handle any bookmarks users may have as all internal links use the new format
// the redirects can be removed at some point in the future when we are no longer concerned about broken bookmarks
if (request.query.q) {
if (isLocationEngland(util.cleanseLocation(request.query.q))) {
return h.redirect(`/${route}`)
}

return locationQueryHandler(request, h)
}
if (rainfallid) {
Expand Down Expand Up @@ -266,7 +271,13 @@ module.exports = [{
}, {
method: 'POST',
path: `/${route}`,
handler: locationQueryHandler,
handler: (request, h) => {
if (isLocationEngland(util.cleanseLocation(request.payload.location))) {
return h.redirect(`/${route}`)
}

return locationQueryHandler(request, h)
},
options: {
validate: {
payload: joi.object({
Expand Down
2 changes: 1 addition & 1 deletion server/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const wreck = require('@hapi/wreck').defaults({
timeout: config.httpTimeoutMs
})
const LocationSearchError = require('./location-search-error')
const ALLOWED_SEARCH_CHARS = 'a-zA-Z0-9\',-.& ()'
const ALLOWED_SEARCH_CHARS = 'a-zA-Z0-9\',-.& ()!'

async function request (method, url, options) {
let res, payload
Expand Down
31 changes: 31 additions & 0 deletions test/routes/alerts-and-warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,37 @@ lab.experiment('Test - /alerts-warnings', () => {
Code.expect(response.statusCode).to.equal(200)
})

lab.test('GET /alerts-and-warnings with non-latin characters should 404', async () => {
stubs.getJson.callsFake(() => data.nonLocationGetJson)
stubs.getFloods.callsFake(() => ({ floods: [] }))

const options = {
method: 'GET',
url: '/alerts-and-warnings?q=你好'
}

const response = await server.inject(options)

Code.expect(response.statusCode).to.equal(404)
})

lab.test('POST /alerts-and-warnings with non-latin characters should return default page', async () => {
stubs.getJson.callsFake(() => {})
stubs.getFloods.callsFake(() => ({ floods: [] }))

const options = {
method: 'POST',
url: '/alerts-and-warnings',
payload: {
location: '你好'
}
}

const response = await server.inject(options)

Code.expect(response.statusCode).to.equal(200)
})

lab.test('POST /alerts-and-warnings with location payload', async () => {
stubs.getJson.callsFake(() => data.warringtonGetJson)
stubs.getIsEngland.callsFake(() => ({ is_england: true }))
Expand Down
31 changes: 31 additions & 0 deletions test/routes/river-and-sea-levels.js
Original file line number Diff line number Diff line change
Expand Up @@ -1290,4 +1290,35 @@ lab.experiment('Test - /river-and-sea-levels', () => {
fullRelatedContentChecker(root)
})
})

lab.test('GET /river-and-sea-levels with non-latin characters should 404', async () => {
stubs.getStations.callsFake(() => [])
stubs.getIsEngland.callsFake(() => ({ is_england: true }))

const options = {
method: 'GET',
url: '/river-and-sea-levels?q=你好'
}

const response = await server.inject(options)

Code.expect(response.statusCode).to.equal(404)
})

lab.test('POST /river-and-sea-levels with non-latin characters should return default page', async () => {
stubs.getStations.callsFake(() => [])
stubs.getIsEngland.callsFake(() => ({ is_england: true }))

const options = {
method: 'POST',
url: '/river-and-sea-levels',
payload: {
location: '你好'
}
}

const response = await server.inject(options)

Code.expect(response.statusCode).to.equal(200)
})
})

0 comments on commit 44d1efc

Please sign in to comment.