Skip to content
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

fix: parse arg typing #36

Merged
merged 3 commits into from
Nov 21, 2024
Merged
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
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@ yarn-error.log*
.DS_Store
*.pem
.vscode/
.eslintcache
.eslintcache
.idea/
32 changes: 16 additions & 16 deletions etc/starter-kitty-validators.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,51 +22,51 @@ export const createUrlSchema: (options?: UrlValidatorOptions) => ZodSchema<URL,
// @public
export interface EmailValidatorOptions {
domains?: {
domain: string;
includeSubdomains?: boolean;
}[];
domain: string
includeSubdomains?: boolean
}[]
}

// @public
export class OptionsError extends Error {
constructor(message: string);
constructor(message: string)
}

// @public
export interface PathValidatorOptions {
basePath: string;
basePath: string
}

// @public
export class RelUrlValidator extends UrlValidator {
constructor(origin: string | URL);
constructor(origin: string | URL)
}

// @public
export class UrlValidationError extends Error {
constructor(message: string);
constructor(message: string)
}

// @public
export class UrlValidator {
constructor(options?: UrlValidatorOptions);
parse(url: string, fallbackUrl: string | URL): URL;
constructor(options?: UrlValidatorOptions)
parse(url: any, fallbackUrl: string | URL): URL
// (undocumented)
parse(url: string): URL;
parse(url: any): URL
// (undocumented)
parse(url: string, fallbackUrl: undefined): URL;
parsePathname(url: string, fallbackUrl: string | URL): string;
parse(url: any, fallbackUrl: undefined): URL
parsePathname(url: any, fallbackUrl: string | URL): string
// (undocumented)
parsePathname(url: string): string;
parsePathname(url: any): string
// (undocumented)
parsePathname(url: string, fallbackUrl: undefined): string;
parsePathname(url: any, fallbackUrl: undefined): string
}

// @public
export interface UrlValidatorOptions {
baseOrigin?: string;
baseOrigin?: string
// Warning: (ae-forgotten-export) The symbol "UrlValidatorWhitelist" needs to be exported by the entry point index.d.ts
whitelist?: UrlValidatorWhitelist;
whitelist?: UrlValidatorWhitelist
}

```
2 changes: 1 addition & 1 deletion packages/validators/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@opengovsg/starter-kitty-validators",
"version": "1.2.9",
"version": "1.2.10",
"main": "./dist/index.js",
"types": "./dist/index.d.ts",
"exports": {
Expand Down
6 changes: 6 additions & 0 deletions packages/validators/src/__tests__/url.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ describe('UrlValidator with default options', () => {
expect(() => validator.parse('javascript&colonalert(/xss/)').protocol).toThrow(UrlValidationError)
expect(() => validator.parse('javascript:alert(/xss/)')).toThrow(UrlValidationError)
})

it('should throw an error when given an invalid type', () => {
expect(() => validator.parse(123)).toThrow(UrlValidationError)
expect(() => validator.parse(undefined)).toThrow(UrlValidationError)
expect(() => validator.parse(['1', '2'])).toThrow(UrlValidationError)
})
})

describe('UrlValidator with custom protocol whitelist', () => {
Expand Down
28 changes: 15 additions & 13 deletions packages/validators/src/url/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import { toSchema } from '@/url/schema'
* @public
*/
export class UrlValidator {
/* eslint-disable @typescript-eslint/no-explicit-any */
// parse functions perform type checking, so input can be any
private schema

/**
Expand Down Expand Up @@ -40,14 +42,14 @@ export class UrlValidator {
/**
* Parses a URL string
*
* @param url - The URL to validate
* @param url - The URL to validate, expects string | URL
* @throws {@link UrlValidationError} if the URL is invalid.
* @returns The URL object if the URL is valid
*
* @internal
*/
#parse(url: string): URL {
const result = this.schema.safeParse(url)
#parse(url: any): URL {
const result = url instanceof URL ? this.schema.safeParse(url.href) : this.schema.safeParse(url)
Copy link

@spaceraccoon spaceraccoon Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick callout here that instanceof checks whether the class is in the entire prototype chain, so:

class MyOwnUrl extends URL {
    constructor(url) {
        super(url)
        this.href = 'https://evil.com'
    }
}

const newUrl = new MyOwnUrl('https://good.com')
console.log(newUrl instanceof URL)  // true
console.log(newUrl.href)    // https://evil.com

In any case this should get caught by safeParse, but just for awareness as there are some rare and esoteric prototype pollution scenarios, and making sure this isn't a hidden assumption.

if (result.success) {
return result.data
}
Expand All @@ -62,17 +64,17 @@ export class UrlValidator {
/**
* Parses a URL string with a fallback option.
*
* @param url - The URL to validate
* @param url - The URL to validate, expects string | URL
* @param fallbackUrl - The fallback URL to return if the URL is invalid.
* @throws {@link UrlValidationError} if the URL is invalid and fallbackUrl is not provided.
* @returns The URL object if the URL is valid, else the fallbackUrl (if provided).
*
* @public
*/
parse(url: string, fallbackUrl: string | URL): URL
parse(url: string): URL
parse(url: string, fallbackUrl: undefined): URL
parse(url: string, fallbackUrl?: string | URL): URL {
parse(url: any, fallbackUrl: string | URL): URL
parse(url: any): URL
parse(url: any, fallbackUrl: undefined): URL
parse(url: any, fallbackUrl?: string | URL): URL {
try {
return this.#parse(url)
} catch (error) {
Expand All @@ -88,17 +90,17 @@ export class UrlValidator {
/**
* Parses a URL string and returns the pathname with a fallback option.
*
* @param url - The URL to validate and extract pathname from
* @param url - The URL to validate and extract pathname from, expects string | URL
* @param fallbackUrl - The fallback URL to use if the URL is invalid.
* @throws {@link UrlValidationError} if the URL is invalid and fallbackUrl is not provided.
* @returns The pathname of the URL or the fallback URL
*
* @public
*/
parsePathname(url: string, fallbackUrl: string | URL): string
parsePathname(url: string): string
parsePathname(url: string, fallbackUrl: undefined): string
parsePathname(url: string, fallbackUrl?: string | URL): string {
parsePathname(url: any, fallbackUrl: string | URL): string
parsePathname(url: any): string
parsePathname(url: any, fallbackUrl: undefined): string
parsePathname(url: any, fallbackUrl?: string | URL): string {
const parsedUrl = fallbackUrl ? this.parse(url, fallbackUrl) : this.parse(url)
if (parsedUrl instanceof URL) return parsedUrl.pathname
return parsedUrl
Expand Down
79 changes: 79 additions & 0 deletions precommit.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
#!/bin/bash

# ANSI colour codes
RED='\033[0;31m'
GREEN='\033[0;32m'
NC='\033[0m' # No Colour

# Function to print error messages
print_error() {
echo -e "${RED}[ERROR ] $1${NC}"
}

# Function to print success messages
print_success() {
echo -e "${GREEN}[SUCCESS] $1${NC}"
}

# Run initial commands
pnpm lint:fix || { print_error "pnpm lint:fix failed"; exit 1; }
pnpm lint || { print_error "pnpm lint failed"; exit 1; }
pnpm build || { print_error "pnpm build failed"; exit 1; }
pnpm format || { print_error "pnpm format failed"; exit 1; }
pnpm format:check || { print_error "pnpm format:check failed"; exit 1; }

# Change directory to packages
cd packages || exit 1

# Variable to track API Extractor failures
api_extractor_failure=0

# Loop through each folder that doesn't end with -config
for dir in */; do
if [[ ! $dir == *-config/ ]]; then
echo "Processing $dir"
(
cd "$dir" || exit 1
pnpx @microsoft/api-extractor run --verbose
)
if [ $? -ne 0 ]; then
print_error "api-extractor failed for $dir"
api_extractor_failure=1
fi
fi
done

if [ $api_extractor_failure -eq 0 ]; then
print_success "All API Extractor operations completed successfully"
else
print_error "One or more API Extractor operations failed"
exit 1
fi

echo "----------------------------------------"
echo "Checking package.json modifications:"
echo "----------------------------------------"

# Check if package.json has been modified in each relevant folder
package_json_modified=0
for dir in */; do
if [[ ! $dir == *-config/ ]]; then
if git diff --quiet HEAD -- "$dir/package.json"; then
echo -e "${RED}package.json in $dir has NOT been modified${NC}"
else
echo -e "${GREEN}package.json in $dir has been modified${NC}"
package_json_modified=1
fi
fi
done

if [ $package_json_modified -eq 0 ]; then
print_error "No package.json was modified"
exit 1
fi

# If we've made it this far, all checks have passed
echo "----------------------------------------"
print_success "Pre-commit checks all succeeded!"
echo "----------------------------------------"
exit 0
Loading