feat: disallow non-ascii characters in desc field (#217)

* feat: disallow non-ascii characters in desc field * feat: change error message to be more informative * feat: add tests for validation * docs: add comment clarifying regex
opengovsg · Jun 25, 2020 · 9d76c1e · 9d76c1e
1 parent 4a7bf91
commit 9d76c1e
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 10 deletions.
diff --git a/src/client/components/UserPage/Drawer/ControlPanel/index.tsx b/src/client/components/UserPage/Drawer/ControlPanel/index.tsx
@@ -25,7 +25,10 @@ import TrailingButton from './widgets/TrailingButton'
 import GoSwitch from './widgets/GoSwitch'
 import useShortLink from './util/shortlink'
 import { removeHttpsProtocol } from '../../../../util/url'
-import { isValidLongUrl } from '../../../../../shared/util/validation'
+import {
+  isValidLongUrl,
+  isPrintableAscii,
+} from '../../../../../shared/util/validation'
 import DownloadButton from './widgets/DownloadButton'
 import helpIcon from '../../../../assets/help-icon.svg'
 import FileInputField from '../../Widgets/FileInputField'
@@ -186,6 +189,9 @@ export default function ControlPanel() {
   const originalContactEmail = shortLinkState?.contactEmail || ''
   const isContactEmailValid =
     !editedContactEmail || emailValidator.match(editedContactEmail)
+  const isDescriptionValid =
+    editedDescription.length <= LINK_DESCRIPTION_MAX_LENGTH &&
+    isPrintableAscii(editedDescription)
 
   // Disposes any current unsaved changes and closes the modal.
   const handleClose = () => {
@@ -509,10 +515,10 @@ export default function ControlPanel() {
                       event.target.value.replace(/(\r\n|\n|\r)/gm, ''),
                     )
                   }
-                  error={editedDescription.length > LINK_DESCRIPTION_MAX_LENGTH}
+                  error={!isDescriptionValid}
                   placeholder=""
                   helperText={
-                    editedDescription.length <= LINK_DESCRIPTION_MAX_LENGTH
+                    isDescriptionValid
                       ? `${editedDescription.length}/${LINK_DESCRIPTION_MAX_LENGTH}`
                       : undefined
                   }
@@ -523,13 +529,13 @@ export default function ControlPanel() {
                 />
                 <CollapsibleMessage
                   type={CollapsibleMessageType.Error}
-                  visible={
-                    editedDescription.length > LINK_DESCRIPTION_MAX_LENGTH
-                  }
+                  visible={!isDescriptionValid}
                   position={CollapsibleMessagePosition.Static}
                   timeout={0}
                 >
-                  {`${editedDescription.length}/200`}
+                  {isPrintableAscii(editedDescription)
+                    ? `${editedDescription.length}/200`
+                    : 'Description should only contain alphanumeric characters and symbols.'}
                 </CollapsibleMessage>
               </>
             }
@@ -540,7 +546,7 @@ export default function ControlPanel() {
           <div className={classes.saveLinkInformationButtonWrapper}>
             <TrailingButton
               disabled={
-                editedDescription.length > LINK_DESCRIPTION_MAX_LENGTH ||
+                !isDescriptionValid ||
                 (editedContactEmail === originalContactEmail &&
                   editedDescription === originalDescription) ||
                 !isContactEmailValid

diff --git a/src/server/api/user/validators.ts b/src/server/api/user/validators.ts
@@ -1,7 +1,11 @@
 import * as Joi from '@hapi/joi'
 import { ACTIVE, INACTIVE } from '../../models/types'
 import blacklist from '../../resources/blacklist'
-import { isHttps, isValidShortUrl } from '../../../shared/util/validation'
+import {
+  isHttps,
+  isPrintableAscii,
+  isValidShortUrl,
+} from '../../../shared/util/validation'
 import { LINK_DESCRIPTION_MAX_LENGTH } from '../../../shared/constants'
 import { isValidGovEmail } from '../../util/email'
 
@@ -53,7 +57,17 @@ export const urlEditSchema = Joi.object({
     file: Joi.object().keys().required(),
   }),
   state: Joi.string().allow(ACTIVE, INACTIVE).only(),
-  description: Joi.string().allow('').max(LINK_DESCRIPTION_MAX_LENGTH),
+  description: Joi.string()
+    .allow('')
+    .max(LINK_DESCRIPTION_MAX_LENGTH)
+    .custom((description: string, helpers) => {
+      if (!isPrintableAscii(description)) {
+        return helpers.message({
+          custom: 'Description must only contain ASCII characters.',
+        })
+      }
+      return description
+    }),
   contactEmail: Joi.string()
     .allow(null)
     .custom((email: string, helpers) => {

diff --git a/src/shared/util/validation.ts b/src/shared/util/validation.ts
@@ -58,3 +58,8 @@ export function isCircularRedirects(url: string, hostname?: string): boolean {
   if (!hostname) return false
   return parse(url).hostname === hostname
 }
+
+export function isPrintableAscii(string: string): boolean {
+  // Only accepts characters from 0x20 to 0x7F
+  return /^[\x20-\x7F]*$/.test(string)
+}
diff --git a/test/shared/util/validation.test.ts b/test/shared/util/validation.test.ts
@@ -131,3 +131,21 @@ describe('Test circular directs check', () => {
     expect(validation.isCircularRedirects(url)).toBe(false)
   })
 })
+
+describe('test isPrintableAscii', () => {
+  it('should return false on non-english language characters', () => {
+    expect(validation.isPrintableAscii('在一个风和日丽的早上')).toBeFalsy()
+    expect(validation.isPrintableAscii('விக்சன')).toBeFalsy()
+    expect(
+      validation.isPrintableAscii('在一个风和日丽的早上, test'),
+    ).toBeFalsy()
+  })
+
+  it('should return true on printable ascii characters', () => {
+    expect(validation.isPrintableAscii('test description')).toBeTruthy()
+    expect(
+      validation.isPrintableAscii("!@#$%^&*()~`-=[];',./\\/*-+"),
+    ).toBeTruthy()
+    expect(validation.isPrintableAscii('aAbBcC')).toBeTruthy()
+  })
+})