Skip to content

Commit

Permalink
Merge pull request #13975 from mozilla/FXA-5793-remove-unique-values-…
Browse files Browse the repository at this point in the history
…from-validation-error-messages

task(content): Remove 'with value $val' from regex validation error m…
  • Loading branch information
dschom authored Aug 31, 2022
2 parents 3d55231 + 4fce838 commit b092b72
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ const { logFlowEvent } = require('../flow-event');
const Url = require('url');
const uuid = require('node-uuid');
const validation = require('../validation');
const {
overrideJoiMessages,
} = require('fxa-shared/sentry/joi-message-overrides');

const {
ACTION: ACTION_TYPE,
Expand Down Expand Up @@ -50,7 +53,7 @@ module.exports = function (config) {
validate: {
// because this endpoint logs data from the query params,
// ensure the query params are all well formed
query: QUERY_PARAM_SCHEMA,
query: overrideJoiMessages(QUERY_PARAM_SCHEMA),
},
process: function (req, res) {
const flowEventData = flowMetrics.create(
Expand Down
5 changes: 4 additions & 1 deletion packages/fxa-content-server/server/lib/routes/post-csp.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ const joi = require('joi');
const logger = require('../logging/log')();
const url = require('url');
const validation = require('../validation');
const {
overrideJoiMessages,
} = require('fxa-shared/sentry/joi-message-overrides');

const INTEGER_TYPE = validation.TYPES.INTEGER;
const STRING_TYPE = validation.TYPES.LONG_STRING;
Expand Down Expand Up @@ -63,7 +66,7 @@ module.exports = function (options = {}) {
method: 'post',
path: options.path,
validate: {
body: BODY_SCHEMA,
body: overrideJoiMessages(BODY_SCHEMA),
},
process: function (req, res) {
res.json({ success: true });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ const joi = require('joi');
const logger = require('../logging/log')('server.post-metrics');
const MetricsCollector = require('../metrics-collector-stderr');
const validation = require('../validation');
const {
overrideJoiMessages,
} = require('fxa-shared/sentry/joi-message-overrides');

const clientMetricsConfig = config.get('client_metrics');
const DISABLE_CLIENT_METRICS_STDERR =
Expand Down Expand Up @@ -163,7 +166,7 @@ module.exports = function () {
method: 'post',
path: '/metrics',
validate: {
body: BODY_SCHEMA,
body: overrideJoiMessages(BODY_SCHEMA),
},
preProcess: function (req, res, next) {
// convert text/plain types to JSON for validation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
const amplitude = require('../amplitude');
const { logFlowEvent } = require('../flow-event');
const validation = require('../validation');
const {
overrideJoiMessages,
} = require('fxa-shared/sentry/joi-message-overrides');

const {
ACTION: ACTION_TYPE,
Expand Down Expand Up @@ -57,7 +60,7 @@ module.exports = function (config) {
validate: {
// because this endpoint logs data from the query params,
// ensure the query params are all well formed
query: QUERY_PARAM_SCHEMA,
query: overrideJoiMessages(QUERY_PARAM_SCHEMA),
},
process: function (req, res) {
const metricsData = req.query;
Expand Down
40 changes: 40 additions & 0 deletions packages/fxa-shared/sentry/joi-message-overrides.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { AnySchema } from 'joi';

/**
* A set of default message overrides. These result in better error resolution in sentry.
*/
export const defaultMessageOverrides = {
// Override some of the message defaults. Here we remove the 'with value {:[.]}'
// portion of the message, because it causes too much fragmentation in our sentry
// errors. These should be applied to any .regex or .pattern joi validator.
// Form more context concerning overriding messages see:
// - https://joi.dev/api/?v=17.6.0#anymessagesmessages
// - https://github.com/hapijs/joi/blob/7aa36666863c1dde7e4eb02a8058e00555a99d54/lib/types/string.js#L718
'string.pattern.base':
'{{#label}} fails to match the required pattern: {{#regex}}',
'string.pattern.name': '{{#label}} fails to match the {{#name}} pattern',
'string.pattern.invert.base':
'{{#label}} matches the inverted pattern: {{#regex}}',
'string.pattern.invert.name':
'{{#label}} matches the inverted {{#name}} pattern',
};

/**
* Applies a set of message overrides to the default joi message formats.
* @param data - Set of joi validators to apply message overrides to to. Note, data is mutated.
* @param overrides - Set of optional overrides, if none are provide the defaultMessageOverrides are used.
* @returns data
*/
export function overrideJoiMessages(
data: Record<string, AnySchema>,
overrides?: Record<string, string>
) {
Object.keys(data).forEach(
(x) => (data[x] = data[x].messages(overrides || defaultMessageOverrides))
);
return data;
}
26 changes: 26 additions & 0 deletions packages/fxa-shared/test/sentry/joi-message-overrides.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { assert } from 'chai';
import joi, { valid } from 'joi';
import { overrideJoiMessages } from '../../sentry/joi-message-overrides';

describe('joi-message-overrides', () => {
it('overrides default message for regex', () => {
const validators = {
test: joi.string().regex(/test/),
};
const result1 = validators.test.validate('foobar').error?.message;

const validators2 = overrideJoiMessages(validators);
const result2 = validators2.test.validate('foobar').error?.message;

assert.exists(validators2);
assert.exists(result1);
assert.exists(result2);
assert.notEqual(result1, result2);
assert.include(result1, 'with value');
assert.notInclude(result2, 'with value');
});
});

0 comments on commit b092b72

Please sign in to comment.