8000 feat(common): throw error for suspicious date patterns by shicks · Pull Request #59798 · angular/angular · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(common): throw error for suspicious date patterns #59798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions goldens/public-api/common/errors.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export const enum RuntimeErrorCode {
// (undocumented)
REQUIRED_INPUT_MISSING = 2954,
// (undocumented)
SUSPICIOUS_DATE_FORMAT = 2300,
// (undocumented)
TOO_MANY_PRELOADED_IMAGES = 2961,
// (undocumented)
TOO_MANY_PRIORITY_ATTRIBUTES = 2966,
Expand Down
3 changes: 3 additions & 0 deletions packages/common/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ export const enum RuntimeErrorCode {
// NgForOf errors
NG_FOR_MISSING_DIFFER = -2200,

// I18n errors
SUSPICIOUS_DATE_FORMAT = 2300,

// Keep 2800 - 2900 for Http Errors.

// Image directive errors
Expand Down
33 changes: 33 additions & 0 deletions packages/common/src/i18n/format_date.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
* found in the LICENSE file at https://angular.dev/license
*/

import {
ɵRuntimeError as RuntimeError,
ɵformatRuntimeError as formatRuntimeError,
} from '@angular/core';

import {
FormatWidth,
FormStyle,
Expand All @@ -24,6 +29,7 @@ import {
Time,
TranslationWidth,
} from './locale_data_api';
import {RuntimeErrorCode} from '../errors';

export const ISO8601_DATE_REGEX =
/^(\d{4,})-?(\d\d)-?(\d\d)(?:T(\d\d)(?::?(\d\d)(?::?(\d\d)(?:\.(\d+))?)?)?(Z|([+-])(\d\d):?(\d\d))?)?$/;
Expand Down Expand Up @@ -104,6 +110,10 @@ export function formatDate(
}
}

if (typeof ngDevMode === 'undefined' || ngDevMode) {
assertValidDateFormat(parts);
}

let dateTimezoneOffset = date.getTimezoneOffset();
if (timezone) {
dateTimezoneOffset = timezoneToOffset(timezone, dateTimezoneOffset);
Expand All @@ -123,6 +133,29 @@ export function formatDate(
return text;
}

/**
* Asserts that the given date format is free from common mistakes. Throws an
* error if one is found (except for the case of all "Y", in which case we just
* log a warning). This should only be called in development mode.
*/
function assertValidDateFormat(parts: string[]) {
if (parts.some((part) => /^Y+$/.test(part)) && !parts.some((part) => /^w+$/.test(part))) {
// "Y" indicates "week-based year", which differs from the actual calendar
// year for a few days around Jan 1 most years. Unless "w" is also
// present (e.g. a date like "2024-W52") this is likely a mistake. Users
// probably meant "y" instead.
const message = `Suspicious use of week-based year "Y" in date pattern "${parts.join(
'',
)}". Did you mean to use calendar year "y" instead?`;
if (parts.length === 1) {
// NOTE: allow "YYYY" with just a warning, since it's used in tests.
console.error(formatRuntimeError(RuntimeErrorCode.SUSPICIOUS_DATE_FORMAT, message));
} else {
throw new RuntimeError(RuntimeErrorCode.SUSPICIOUS_DATE_FORMAT, message);
}
}
}

/**
* Create a new Date object with the given date value, and the time set to midnight.
*
Expand Down
40 changes: 27 additions & 13 deletions packages/common/test/i18n/format_date_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,14 @@ describe('Format date', () => {
BBBBB: 'at night',
};

// Suppress console warnings for 'YYYY' patterns.
const consoleError = console.error;
spyOn(console, 'error').and.callFake((...args: unknown[]) => {
if (!/Suspicious use of week-based year/.test(String(args))) {
consoleError(...args);
}
});

Object.keys(dateFixtures).forEach((pattern: string) => {
expectDateFormatAs(date, pattern, dateFixtures[pattern]);
});
Expand Down Expand Up @@ -450,17 +458,23 @@ describe('Format date', () => {

// https://github.com/angular/angular/issues/38739
it('should return correct ISO 8601 week-numbering year for dates close to year end/beginning', () => {
expect(formatDate('2013-12-27', 'YYYY', 'en')).toEqual('2013');
expect(formatDate('2013-12-29', 'YYYY', 'en')).toEqual('2013');
expect(formatDate('2013-12-31', 'YYYY', 'en')).toEqual('2014');
expect(formatDate('2013-12-27', `YYYY 'W'ww`, 'en')).toEqual('2013 W52');
expect(formatDate('2013-12-29', `YYYY 'W'ww`, 'en')).toEqual('2013 W52');
expect(formatDate('2013-12-31', `YYYY 'W'ww`, 'en')).toEqual('2014 W01');

// Dec. 31st is a Sunday, last day of the last week of 2023
expect(formatDate('2023-12-31', 'YYYY', 'en')).toEqual('2023');
expect(formatDate('2023-12-31', `YYYY 'W'ww`, 'en')).toEqual('2023 W52');

expect(formatDate('2010-01-02', 'YYYY', 'en')).toEqual('2009');
expect(formatDate('2010-01-04', 'YYYY', 'en')).toEqual('2010');
expect(formatDate('0049-01-01', 'YYYY', 'en')).toEqual('0048');
expect(formatDate('0049-01-04', 'YYYY', 'en')).toEqual('0049');
expect(formatDate('2010-01-02', `YYYY 'W'ww`, 'en')).toEqual('2009 W53');
expect(formatDate('2010-01-04', `YYYY 'W'ww`, 'en')).toEqual('2010 W01');
expect(formatDate('0049-01-01', `YYYY 'W'ww`, 'en')).toEqual('0048 W53');
expect(formatDate('0049-01-04', `YYYY 'W'ww`, 'en')).toEqual('0049 W01');
});

it('should throw an error when using YYYY incorrectly', () => {
expect(() => formatDate('2013-12-31', `YYYY/MM/dd`, ɵDEFAULT_LOCALE_ID)).toThrowError(
/.*Suspicious use of week-based year "Y".*/,
);
});

// https://github.com/angular/angular/issues/53813
Expand All @@ -480,11 +494,11 @@ describe('Format date', () => {

// https://github.com/angular/angular/issues/40377
it('should format date with year between 0 and 99 correctly', () => {
expect(formatDate('0098-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0098');
expect(formatDate('0099-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0099');
expect(formatDate('0100-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0100');
expect(formatDate('0001-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0001');
expect(formatDate('0000-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0000');
expect(formatDate('0098-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0098 W02');
expect(formatDate('0099-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0099 W02');
expect(formatDate('0100-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0100 W02');
expect(formatDate('0001-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0001 W02');
expect(formatDate('0000-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0000 W02');
});

// https://github.com/angular/angular/issues/26922
Expand Down
0