fix: add client-side validation for Firestore writes (#154) - #768
fix: add client-side validation for Firestore writes (#154)#768saurabhhhcodes wants to merge 1 commit into
Conversation
…singh#154) Event and profile payloads were written straight to Firestore with no client-side checks, so invalid types, oversized strings and malformed links only failed (or corrupted data) at the rules layer with generic permission errors. - lib/validators.js: dependency-free validators mirroring the server rules (validateEventShape): title/description length limits, ISO date ordering, price/capacity bounds, https-only URLs; validateEventInput and validateProfileInput return { valid, errors } - CreateEvent.js: event payload validated before the create/update write, with a friendly error alert listing all problems - 9 tests covering valid/invalid payloads, types, bounds and URL rules
|
📝 WalkthroughWalkthroughChangesInput validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CreateEvent
participant validateEventInput
participant Firestore
CreateEvent->>validateEventInput: validate assembled event payload
alt invalid payload
validateEventInput-->>CreateEvent: return validation errors
CreateEvent-->>CreateEvent: show Invalid Input alert
else valid payload
validateEventInput-->>CreateEvent: return valid
CreateEvent->>Firestore: update or create event
end
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/src/lib/__tests__/validators.test.jsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. app/src/lib/validators.jsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. app/src/screens/CreateEvent.jsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/lib/__tests__/validators.test.js`:
- Line 1: Update the import statement in the validator tests to import
validateEventInput, validateProfileInput, and isHttpUrl from the parent
directory using ../validators instead of ./validators.
In `@app/src/lib/validators.js`:
- Around line 69-72: Update the capacity validation condition in the visible
validator to also require Number.isInteger(data.capacity), preserving the
existing range and finite-number checks. Add or update the validator tests to
verify that a decimal capacity such as 1.5 is rejected.
- Around line 103-115: Update the optional bio and Instagram validation checks
in the profile validator to require string values before accessing bio.length or
applying the Instagram pattern. Ensure null and other non-string values are
rejected with the appropriate existing validation errors, while valid strings
retain the current length and format rules.
- Around line 38-90: Reduce cognitive complexity in validateEventInput by
extracting the date, numeric, and link validation branches into focused helper
functions, then append their errors in the same order currently produced.
Preserve all existing validation rules, messages, and ordering while keeping
validateEventInput responsible for combining the helper results.
- Around line 81-87: Update the bannerUrl validation condition in the relevant
validator to rely solely on !isHttpUrl(data.bannerUrl), removing the
/^https?:\/\// special case so HTTP URLs are rejected while valid URLs continue
through the existing error handling.
- Around line 30-31: Update isHttpUrl to parse the value with the URL parser and
reject parsing failures, non-https protocols, or empty hostnames while retaining
the existing string and maxLength checks. Add a regression test covering the
malformed value “https://?” and ensure it is rejected.
In `@app/src/screens/CreateEvent.js`:
- Around line 482-487: Move or add a preflight call to validateEventInput before
uploadImage() and optional Meet-link generation, using the form-controlled event
fields available at that stage; show the existing invalid-input alert, clear
loading, and return on failure. Retain the current validation after derived
values are added so the final eventData remains validated before persistence.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c540625e-8089-4f58-add0-1d6c8f0f7483
📒 Files selected for processing (3)
app/src/lib/__tests__/validators.test.jsapp/src/lib/validators.jsapp/src/screens/CreateEvent.js
| @@ -0,0 +1,91 @@ | |||
| import { validateEventInput, validateProfileInput, isHttpUrl } from './validators'; | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# List validator modules under the relevant library directory.
fd -a -t f '^validators\.js$' app/src/libRepository: roshankumar0036singh/Uni-Event
Length of output: 213
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- test directory files ---'
find app/src/lib/__tests__ -maxdepth 1 -type f -printf '%f\n' | sort
printf '%s\n' '--- import and validator references ---'
rg -n --no-heading "from ['\"]\\.?\\.?/validators|require\\(['\"]\\.?\\.?/validators|validateEventInput|validateProfileInput|isHttpUrl" \
app/src/lib/__tests__ app/src/lib/validators.js
printf '%s\n' '--- relevant test file ---'
cat -n app/src/lib/__tests__/validators.test.js | sed -n '1,80p'
printf '%s\n' '--- nearby validator paths ---'
find app/src/lib -maxdepth 3 \( -type f -o -type l \) -iname 'validators*' -print | sortRepository: roshankumar0036singh/Uni-Event
Length of output: 8341
Import the validator module from its parent directory.
./validators resolves to the test directory, where no sibling module exists. Use ../validators.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/lib/__tests__/validators.test.js` at line 1, Update the import
statement in the validator tests to import validateEventInput,
validateProfileInput, and isHttpUrl from the parent directory using
../validators instead of ./validators.
| export const isHttpUrl = (value, maxLength = LIMITS.URL_MAX) => | ||
| typeof value === 'string' && value.length <= maxLength && /^https:\/\/.+/.test(value); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect URL API/polyfill usage before adopting a parser-based implementation.
fd -a -t f '^package\.json$' . -x sh -c 'echo "--- $1"; rg -n "react-native-url-polyfill|whatwg-url|\\bURL\\b" "$1" || true' sh {}
rg -n -C 2 'isHttpUrl|new URL|react-native-url-polyfill|whatwg-url' app/srcRepository: roshankumar0036singh/Uni-Event
Length of output: 2976
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in $(fd -a -t f '^package\.json$' .); do
echo "--- $f"
cat "$f"
done
echo "--- app/src/lib/validators.js"
cat -n app/src/lib/validators.js
echo "--- app/src/lib/__tests__/validators.test.js"
cat -n app/src/lib/__tests__/validators.test.js
echo "--- runtime/config references"
rg -n -C 2 'react-native|expo|engines|global\.URL|new URL|URL\(' --glob '!**/node_modules/**' --glob '!**/dist/**' .Repository: roshankumar0036singh/Uni-Event
Length of output: 50387
🏁 Script executed:
node - <<'JS'
const values = ['https://?', 'https://', 'https://.', 'https://example.com/a'];
const re = /^https:\/\/.+/;
for (const value of values) {
let parsed;
try {
parsed = new URL(value);
} catch {
parsed = null;
}
console.log(JSON.stringify({
value,
currentValidator: re.test(value),
parsedProtocol: parsed?.protocol ?? null,
parsedHostname: parsed?.hostname ?? null,
parserAccepts: Boolean(parsed && parsed.protocol === 'https:' && parsed.hostname),
}));
}
console.log({globalURLType: typeof URL});
JSRepository: roshankumar0036singh/Uni-Event
Length of output: 672
🌐 Web query:
React Native 0.76.9 global URL API Hermes URL constructor supported
💡 Result:
Yes, in React Native 0.76.9, the global URL API is supported in the Hermes JavaScript engine [1]. React Native provides a built-in implementation of the URL and URLSearchParams classes that is available globally [1]. While older versions of React Native had a partially implemented URL class with missing methods and properties, version 0.76.9 includes updates that added standard properties (such as hash, host, hostname, and pathname) and methods (such as get, set, and delete) [1]. Key points regarding the current status: - Native Availability: The URL constructor is exposed globally within the React Native environment, including when using Hermes [2][1]. - Updates in 0.76.9: The 0.76.9 release includes specific improvements to the URL class, such as adding missing standard properties and fixing type definitions for the constructor [1]. - Known Limitations: Despite ongoing improvements, the native implementation is often described as a lightweight, partial polyfill rather than a full, spec-compliant implementation of the WHATWG URL standard [3]. Users may still encounter edge cases or minor behavioral differences compared to web browser implementations (e.g., specific issues with relative path resolution or trailing slashes in some scenarios) [4][5]. - Alternatives: If you require a strictly spec-compliant or more robust implementation, community-maintained polyfills like react-native-url-polyfill remain available and compatible with Hermes [3]. In summary, the global URL API is functional and significantly more complete in React Native 0.76.9 than in previous releases, making it suitable for most standard use cases [1].
Citations:
- 1: https://npmx.dev/package-changelog/react-native/v/0.76.9
- 2: Implement Missing Methods in URL Class to Handle Base URL and Relative Path Combinations react/react-native#45055
- 3: https://github.com/charpeni/react-native-url-polyfill/
- 4: URL constructor not spec compliant react/react-native#49856
- 5: URL class constructor incorrectly adds final forward slash to URL react/react-native#54242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
from pathlib import Path
for path in Path('.').rglob('package.json'):
if any(part in {'node_modules', 'dist', 'build'} for part in path.parts):
continue
data = json.loads(path.read_text())
print(f'--- {path}')
for section in ('engines', 'dependencies', 'devDependencies', 'peerDependencies'):
values = data.get(section, {})
if section == 'engines':
print(section, values)
else:
print(section, {k: v for k, v in values.items()
if 'url' in k.lower() or 'react-native' in k.lower()
or k in {'react', 'expo', 'hermes-engine'}})
PY
echo "--- URL-related lockfile package names"
rg -n '"(react-native-url-polyfill|whatwg-url|url|url-parse|react-native)"' app/package-lock.json package-lock.json 2>/dev/null | head -80 || true
echo "--- focused URL references outside generated/dependency files"
rg -n -C 2 'isHttpUrl|new URL|react-native-url-polyfill|whatwg-url|global\.URL' app/src app/App.js app/index.js app/package.json app/babel.config.js app/metro.config.js 2>/dev/null || trueRepository: roshankumar0036singh/Uni-Event
Length of output: 10723
Parse the URL before accepting it. Reject values when parsing fails, the protocol is not https:, or the hostname is empty. The current regex accepts malformed values such as https://?; add a regression test for this case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/lib/validators.js` around lines 30 - 31, Update isHttpUrl to parse
the value with the URL parser and reject parsing failures, non-https protocols,
or empty hostnames while retaining the existing string and maxLength checks. Add
a regression test covering the malformed value “https://?” and ensure it is
rejected.
| export const validateEventInput = (data = {}) => { | ||
| const errors = []; | ||
|
|
||
| if (!isStringOfLength(data.title, LIMITS.TITLE_MIN, LIMITS.TITLE_MAX)) { | ||
| errors.push( | ||
| `Title must be between ${LIMITS.TITLE_MIN} and ${LIMITS.TITLE_MAX} characters.`, | ||
| ); | ||
| } | ||
|
|
||
| if (!isNonEmptyString(data.description) || data.description.length > LIMITS.DESCRIPTION_MAX) { | ||
| errors.push( | ||
| `Description is required and must not exceed ${LIMITS.DESCRIPTION_MAX} characters.`, | ||
| ); | ||
| } | ||
|
|
||
| if (!isIsoDateString(data.startAt)) { | ||
| errors.push('Start date must be a valid date.'); | ||
| } | ||
| if (!isIsoDateString(data.endAt)) { | ||
| errors.push('End date must be a valid date.'); | ||
| } | ||
| if (isIsoDateString(data.startAt) && isIsoDateString(data.endAt)) { | ||
| if (new Date(data.endAt) <= new Date(data.startAt)) { | ||
| errors.push('End date must be after the start date.'); | ||
| } | ||
| } | ||
|
|
||
| if (!isFiniteNumber(data.price, 0, LIMITS.PRICE_MAX)) { | ||
| errors.push(`Price must be a number between 0 and ${LIMITS.PRICE_MAX}.`); | ||
| } | ||
|
|
||
| if (data.capacity !== null && data.capacity !== undefined) { | ||
| if (!isFiniteNumber(data.capacity, 1, LIMITS.CAPACITY_MAX)) { | ||
| errors.push(`Capacity must be a whole number between 1 and ${LIMITS.CAPACITY_MAX}.`); | ||
| } | ||
| } | ||
|
|
||
| if (data.meetLink && !isHttpUrl(data.meetLink)) { | ||
| errors.push('Meet link must be an https:// URL.'); | ||
| } | ||
| if (data.registrationLink && !isHttpUrl(data.registrationLink)) { | ||
| errors.push('Registration link must be an https:// URL.'); | ||
| } | ||
| if ( | ||
| data.bannerUrl && | ||
| !isHttpUrl(data.bannerUrl) && | ||
| /^https?:\/\//.test(data.bannerUrl) === false | ||
| ) { | ||
| errors.push('Banner URL must be an https:// URL.'); | ||
| } | ||
|
|
||
| return { valid: errors.length === 0, errors }; | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Reduce validateEventInput complexity.
SonarCloud reports cognitive complexity 19, but the configured maximum is 15. Extract date, numeric, and link checks into focused helpers. Preserve the current error order.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 38-38: Refactor this function to reduce its Cognitive Complexity from 19 to the 15 allowed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/lib/validators.js` around lines 38 - 90, Reduce cognitive complexity
in validateEventInput by extracting the date, numeric, and link validation
branches into focused helper functions, then append their errors in the same
order currently produced. Preserve all existing validation rules, messages, and
ordering while keeping validateEventInput responsible for combining the helper
results.
Source: Linters/SAST tools
| if (data.capacity !== null && data.capacity !== undefined) { | ||
| if (!isFiniteNumber(data.capacity, 1, LIMITS.CAPACITY_MAX)) { | ||
| errors.push(`Capacity must be a whole number between 1 and ${LIMITS.CAPACITY_MAX}.`); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require an integer capacity.
isFiniteNumber(1.5, 1, LIMITS.CAPACITY_MAX) returns true. This permits fractional capacities although the error message requires a whole number. Add Number.isInteger(data.capacity) and cover a decimal capacity in the tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/lib/validators.js` around lines 69 - 72, Update the capacity
validation condition in the visible validator to also require
Number.isInteger(data.capacity), preserving the existing range and finite-number
checks. Add or update the validator tests to verify that a decimal capacity such
as 1.5 is rejected.
| if ( | ||
| data.bannerUrl && | ||
| !isHttpUrl(data.bannerUrl) && | ||
| /^https?:\/\//.test(data.bannerUrl) === false | ||
| ) { | ||
| errors.push('Banner URL must be an https:// URL.'); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject HTTP banner URLs.
The ^https?:\/\/ condition makes http://example.com/banner.png pass validation. Replace this special case with the same !isHttpUrl(data.bannerUrl) check used for the other links.
Proposed fix
- if (
- data.bannerUrl &&
- !isHttpUrl(data.bannerUrl) &&
- /^https?:\/\//.test(data.bannerUrl) === false
- ) {
+ if (data.bannerUrl && !isHttpUrl(data.bannerUrl)) {
errors.push('Banner URL must be an https:// URL.');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| data.bannerUrl && | |
| !isHttpUrl(data.bannerUrl) && | |
| /^https?:\/\//.test(data.bannerUrl) === false | |
| ) { | |
| errors.push('Banner URL must be an https:// URL.'); | |
| } | |
| if (data.bannerUrl && !isHttpUrl(data.bannerUrl)) { | |
| errors.push('Banner URL must be an https:// URL.'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/lib/validators.js` around lines 81 - 87, Update the bannerUrl
validation condition in the relevant validator to rely solely on
!isHttpUrl(data.bannerUrl), removing the /^https?:\/\// special case so HTTP
URLs are rejected while valid URLs continue through the existing error handling.
| if (data.bio !== undefined && data.bio.length > 600) { | ||
| errors.push('Bio must not exceed 600 characters.'); | ||
| } | ||
| if ( | ||
| data.instagram !== undefined && | ||
| data.instagram && | ||
| !/^[a-zA-Z0-9._]{1,30}$/.test(data.instagram) | ||
| ) { | ||
| errors.push('Instagram handle contains invalid characters.'); | ||
| } | ||
| if (data.linkedin !== undefined && data.linkedin && !isHttpUrl(data.linkedin)) { | ||
| errors.push('LinkedIn URL must be an https:// URL.'); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate optional profile field types before reading them.
bio: null throws at data.bio.length. Non-string values such as bio: 123 and instagram: 123 can also pass validation. Check for strings before reading .length or applying the handle pattern.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/lib/validators.js` around lines 103 - 115, Update the optional bio
and Instagram validation checks in the profile validator to require string
values before accessing bio.length or applying the Instagram pattern. Ensure
null and other non-string values are rejected with the appropriate existing
validation errors, while valid strings retain the current length and format
rules.
| const validation = validateEventInput(eventData); | ||
| if (!validation.valid) { | ||
| Alert.alert('Invalid Input', validation.errors.join('\n')); | ||
| setLoading(false); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Validate user-controlled fields before external side effects.
This validation runs after uploadImage() and optional Meet-link generation. An invalid registration link, title, or capacity can therefore upload a Storage object or create a calendar event before the alert stops the Firestore write.
Run a preflight validation on the form-controlled fields before image upload and Meet generation. Keep this final validation after derived values are added.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/screens/CreateEvent.js` around lines 482 - 487, Move or add a
preflight call to validateEventInput before uploadImage() and optional Meet-link
generation, using the form-controlled event fields available at that stage; show
the existing invalid-input alert, clear loading, and return on failure. Retain
the current validation after derived values are added so the final eventData
remains validated before persistence.
1 participant
Closes #154
Problem
Event and profile payloads were written to Firestore with no client-side validation — invalid types, oversized strings, and malformed links only failed at the rules layer with generic permission errors (or worse, partially corrupted data).
Changes
app/src/lib/validators.js(new, no new dependencies): validators that mirror the existing server-side rules (validateEventShapeinfirestore.rules):validateEventInput— title (3-200 chars), required description (max 5000), ISO date + end-after-start ordering, price (0-100000), capacity (1-10000), https-only meet/registration/banner URLsvalidateProfileInput— display name length, bio cap, instagram handle charset, https-only LinkedIn{ valid, errors }for friendly messagesapp/src/screens/CreateEvent.js: event payload validated before both create and update writes; invalid input shows an alert listing every problem instead of writing.app/src/lib/__tests__/validators.test.js: 9 tests — valid payloads, short/long titles, missing/oversized descriptions, invalid date order, price/capacity bounds, http/javascript links rejected, null/optional values allowed.Requests the same protection for user/profile writes as a follow-up once the event path lands.
Summary by CodeRabbit
New Features
Tests