From 506f3f80bc406f33e6d9702ce0a495218129403d Mon Sep 17 00:00:00 2001 From: Tristan Carel Date: Fri, 21 Aug 2026 02:32:20 +0200 Subject: [PATCH 1/2] fix(organization): return 400 instead of 500 on over-long postalCode PUT /api/v1/organizations/:id passed postalCode straight to TGrotto.updateOne(); a value longer than the column limit (varchar(10)) made Waterline throw E_INVALID_VALUES_TO_SET, surfacing as an unhandled 500 instead of a client error. Add validatePostalCodeLength (built on the existing validateStringLength util, mirroring nameValidation) and guard both the create and the update controllers before any write. The check runs on the trimmed value from GrottoService.getConvertedDataFromClientRequest so a postal code that fits once trimmed is still accepted. Closes #1774 --- api/controllers/v1/organization/create.js | 9 +++++ api/controllers/v1/organization/update.js | 10 +++++ api/utils/postalCodeValidation.js | 21 ++++++++++ .../2_utils/postalCodeValidation.test.js | 37 ++++++++++++++++++ .../4_routes/Organization/create.test.js | 21 ++++++++++ .../4_routes/Organization/update.test.js | 38 +++++++++++++++++++ 6 files changed, 136 insertions(+) create mode 100644 api/utils/postalCodeValidation.js create mode 100644 test/integration/2_utils/postalCodeValidation.test.js diff --git a/api/controllers/v1/organization/create.js b/api/controllers/v1/organization/create.js index 5d11eb3e9..5b8684e8c 100644 --- a/api/controllers/v1/organization/create.js +++ b/api/controllers/v1/organization/create.js @@ -2,6 +2,9 @@ const ControllerService = require('../../../services/ControllerService'); const GrottoService = require('../../../services/GrottoService'); const { toOrganization } = require('../../../services/mapping/converters'); const { validateNameLength } = require('../../../utils/nameValidation'); +const { + validatePostalCodeLength, +} = require('../../../utils/postalCodeValidation'); module.exports = async (req, res) => { // Check params @@ -23,6 +26,12 @@ module.exports = async (req, res) => { dateInscription: new Date(), }; + // Validate postalCode length (checked on the trimmed value) + const postalCodeError = validatePostalCodeLength(cleanedData.postalCode); + if (postalCodeError) { + return res.badRequest(postalCodeError); + } + const nameData = { author: req.token.id, language: req.param('name').language, diff --git a/api/controllers/v1/organization/update.js b/api/controllers/v1/organization/update.js index 248bbe9f8..dfab240ce 100644 --- a/api/controllers/v1/organization/update.js +++ b/api/controllers/v1/organization/update.js @@ -4,6 +4,9 @@ const NotificationService = require('../../../services/NotificationService'); const EnrichmentQueueService = require('../../../services/EnrichmentQueueService'); const { toOrganization } = require('../../../services/mapping/converters'); const { validateNameLength } = require('../../../utils/nameValidation'); +const { + validatePostalCodeLength, +} = require('../../../utils/postalCodeValidation'); module.exports = async (req, res) => { // Check if organization exists @@ -36,6 +39,13 @@ module.exports = async (req, res) => { coordinatesChanged = true; } + // Validate postalCode length before any write (checked on the trimmed value, + // so a value that fits once trimmed is not rejected) + const postalCodeError = validatePostalCodeLength(cleanedData.postalCode); + if (postalCodeError) { + return res.badRequest(postalCodeError); + } + // Validate name length const nameText = req.body.name?.text; const nameError = validateNameLength(nameText); diff --git a/api/utils/postalCodeValidation.js b/api/utils/postalCodeValidation.js new file mode 100644 index 000000000..f9d63e2ac --- /dev/null +++ b/api/utils/postalCodeValidation.js @@ -0,0 +1,21 @@ +const { validateStringLength } = require('./stringLengthValidation'); + +const POSTAL_CODE_MAX_LENGTH = 10; // TGrotto.postalCode maxLength + +/** + * Validates that a postal code does not exceed the DB column limit. + * Returns an error message if invalid, or null if valid. + * + * @param {string} postalCode - The postal code to validate + * @returns {string|null} Error message or null + */ +const validatePostalCodeLength = (postalCode) => { + const error = validateStringLength( + 'Postal code', + postalCode, + POSTAL_CODE_MAX_LENGTH + ); + return error ? error.message : null; +}; + +module.exports = { POSTAL_CODE_MAX_LENGTH, validatePostalCodeLength }; diff --git a/test/integration/2_utils/postalCodeValidation.test.js b/test/integration/2_utils/postalCodeValidation.test.js new file mode 100644 index 000000000..1836605d0 --- /dev/null +++ b/test/integration/2_utils/postalCodeValidation.test.js @@ -0,0 +1,37 @@ +const should = require('should'); +const { + POSTAL_CODE_MAX_LENGTH, + validatePostalCodeLength, +} = require('../../../api/utils/postalCodeValidation'); + +describe('postalCodeValidation', () => { + describe('validatePostalCodeLength', () => { + it('should expose the TGrotto.postalCode maxLength', () => { + should(POSTAL_CODE_MAX_LENGTH).equal(10); + }); + + it('should return null for a short postal code', () => { + should(validatePostalCodeLength('84000')).be.null(); + }); + + it('should return null for a postal code at exactly the limit', () => { + should(validatePostalCodeLength('1234567890')).be.null(); + }); + + // Value reported in https://github.com/GrottoCenter/grottocenter-api/issues/1774 + it('should return an error message for a too long postal code', () => { + const result = validatePostalCodeLength('4400 Flémalle'); + should(result).be.a.String(); + should(result).containEql('Postal code'); + should(result).containEql('exceeds maximum length of 10'); + should(result).containEql('got 13'); + should(result).containEql('3 over limit'); + }); + + it('should return null for null, undefined and non-string values', () => { + should(validatePostalCodeLength(null)).be.null(); + should(validatePostalCodeLength(undefined)).be.null(); + should(validatePostalCodeLength(84000)).be.null(); + }); + }); +}); diff --git a/test/integration/4_routes/Organization/create.test.js b/test/integration/4_routes/Organization/create.test.js index b91a6bb58..933ae00d2 100644 --- a/test/integration/4_routes/Organization/create.test.js +++ b/test/integration/4_routes/Organization/create.test.js @@ -66,5 +66,26 @@ describe('Organization features', () => { }); }).timeout(4000); }); + + // https://github.com/GrottoCenter/grottocenter-api/issues/1774 + describe('Invalid data', () => { + it('should return 400 when postalCode is too long', (done) => { + supertest(sails.hooks.http.app) + .post('/api/v1/organizations') + .send({ + name: { text: 'Organisation Flémalle', language: 'fr' }, + postalCode: '4400 Flémalle', + }) + .set('Authorization', userToken) + .set('Content-type', 'application/json') + .set('Accept', 'application/json') + .expect(400) + .end((err, res) => { + if (err) return done(err); + should(res.body.message).containEql('Postal code'); + return done(); + }); + }); + }); }); }); diff --git a/test/integration/4_routes/Organization/update.test.js b/test/integration/4_routes/Organization/update.test.js index 8fa1b47a5..75b418a3c 100644 --- a/test/integration/4_routes/Organization/update.test.js +++ b/test/integration/4_routes/Organization/update.test.js @@ -198,6 +198,44 @@ describe('Organization features', () => { .send({ name: { text: 'A'.repeat(500) } }) .expect(400, done); }); + + // https://github.com/GrottoCenter/grottocenter-api/issues/1774 + it('should return 400 when postalCode is too long', (done) => { + supertest(sails.hooks.http.app) + .put('/api/v1/organizations/1') + .set('Authorization', userToken) + .set('Content-type', 'application/json') + .set('Accept', 'application/json') + .send({ postalCode: '4400 Flémalle' }) + .expect(400) + .end(async (err, res) => { + if (err) return done(err); + try { + should(res.body.message).containEql('Postal code'); + // Nothing must have been persisted + const organization = await TGrotto.findOne({ id: 1 }); + should(organization.postalCode).not.equal('4400 Flémalle'); + return done(); + } catch (testErr) { + return done(testErr); + } + }); + }); + + it('should return 200 when postalCode is exactly 10 characters', (done) => { + supertest(sails.hooks.http.app) + .put('/api/v1/organizations/1') + .set('Authorization', userToken) + .set('Content-type', 'application/json') + .set('Accept', 'application/json') + .send({ postalCode: '1234567890' }) + .expect(200) + .end((err, res) => { + if (err) return done(err); + should(res.body.postalCode).equal('1234567890'); + return done(); + }); + }); }); }); }); From dfb70a49b95f9aa6530d099b405c10964bc145e3 Mon Sep 17 00:00:00 2001 From: Tristan Carel Date: Mon, 24 Aug 2026 22:53:48 +0200 Subject: [PATCH 2/2] fix(organization): address PR #1780 review feedback - swagger: type postalCode as string (was number) in the POST query params and document the maxLength: 10 constraint on both POST and PUT, mirroring how name.text documents its own limit; mention the new case in the 400 response descriptions. - update.test.js: reset org 1 postalCode to its fixture value after the "exactly 10 characters" test, consistent with the TName resets in the same file. - postalCodeValidation.test.js: cover the empty string explicitly. --- assets/swaggerV1.yaml | 9 ++++++--- .../2_utils/postalCodeValidation.test.js | 3 ++- .../4_routes/Organization/update.test.js | 13 ++++++++++--- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/assets/swaggerV1.yaml b/assets/swaggerV1.yaml index 2a9677cf3..7ea996899 100644 --- a/assets/swaggerV1.yaml +++ b/assets/swaggerV1.yaml @@ -5931,7 +5931,8 @@ paths: - name: postalCode in: query schema: - type: number + type: string + maxLength: 10 - name: region in: query schema: @@ -5954,7 +5955,7 @@ paths: items: $ref: '#/components/schemas/Organization' '400': - description: You must provide at least a name to create an organization. + description: You must provide at least a name to create an organization, or the postal code is too long. '403': description: You are not authorized to create an organization. @@ -6040,6 +6041,8 @@ paths: type: string postalCode: type: string + description: Postal code (max 10 characters) + maxLength: 10 region: type: string url: @@ -6066,7 +6069,7 @@ paths: schema: $ref: '#/components/schemas/Organization' 400: - description: Validation error (name too long, language null or not found) + description: Validation error (name too long, postal code too long, language null or not found) 403: description: You are not authorized to update an organization. 404: diff --git a/test/integration/2_utils/postalCodeValidation.test.js b/test/integration/2_utils/postalCodeValidation.test.js index 1836605d0..85fd6bc01 100644 --- a/test/integration/2_utils/postalCodeValidation.test.js +++ b/test/integration/2_utils/postalCodeValidation.test.js @@ -28,9 +28,10 @@ describe('postalCodeValidation', () => { should(result).containEql('3 over limit'); }); - it('should return null for null, undefined and non-string values', () => { + it('should return null for null, undefined, empty string and non-string values', () => { should(validatePostalCodeLength(null)).be.null(); should(validatePostalCodeLength(undefined)).be.null(); + should(validatePostalCodeLength('')).be.null(); should(validatePostalCodeLength(84000)).be.null(); }); }); diff --git a/test/integration/4_routes/Organization/update.test.js b/test/integration/4_routes/Organization/update.test.js index 75b418a3c..f2e713550 100644 --- a/test/integration/4_routes/Organization/update.test.js +++ b/test/integration/4_routes/Organization/update.test.js @@ -230,10 +230,17 @@ describe('Organization features', () => { .set('Accept', 'application/json') .send({ postalCode: '1234567890' }) .expect(200) - .end((err, res) => { + .end(async (err, res) => { if (err) return done(err); - should(res.body.postalCode).equal('1234567890'); - return done(); + try { + should(res.body.postalCode).equal('1234567890'); + + // Reset + await TGrotto.updateOne({ id: 1 }).set({ postalCode: '92130' }); + return done(); + } catch (testErr) { + return done(testErr); + } }); }); });