fix panic #314
fix panic #314
Conversation
WalkthroughThe pull request introduces modifications to two HTML templates, Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
templates/notification.html(1 hunks)templates/user_list.html(2 hunks)
🔇 Additional comments (1)
templates/user_list.html (1)
Line range hint
112-139: Verify role handling in the backendLet's verify how roles are processed in the backend to ensure our fix aligns with the implementation.
✅ Verification successful
Role values in template match backend implementation
The role values in the template correctly align with the backend implementation:
For site-wide roles (
Roleenum):
- Banned = 0
- Standard = 10
- Senior = 100
- Admin = 255
For inn-specific roles (
InnRoleenum):
- Pending = 1
- Rejected = 2
- Limited = 3
- Intern = 4
- Fellow = 5
- Mod = 7
- Super = 10
The template's role comparison values match exactly with the backend's role definitions, and the form submission is properly handled by the
role_posthandler which validates the roles and permissions appropriately.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for role-related type definitions and processing echo "Searching for role type definitions..." rg -t rust "type.*Role|struct.*Role|enum.*Role" echo "Searching for role processing in form handlers..." rg -t rust "fn.*role.*|pub.*role.*" echo "Searching for role-related conversions..." rg -t rust "\.parse.*role|from_str.*role|into.*role"Length of output: 841
Script:
#!/bin/bash # Let's examine the role enums and form handling implementation echo "Role enums and form handling implementation:" rg -A 20 "pub\(super\) enum Role" src/controller/user.rs rg -A 20 "pub\(super\) enum InnRole" src/controller/user.rs rg -A 20 "pub\(crate\) struct FormRole" src/controller/user.rs echo -e "\nRole post handler implementation:" rg -A 30 "pub\(crate\) async fn role_post" src/controller/user.rs echo -e "\nUpdate role implementation:" rg -A 20 "fn update_role" src/controller/user.rsLength of output: 4142
| <option selected value="Pending">{{ "pending"|l10n(page_data.lang) }}</option> | ||
| <option value="Rejected">{{ "rejected"|l10n(page_data.lang) }}</option> | ||
| <option value="Limited">{{ "limited"|l10n(page_data.lang) }}</option> | ||
| <option value="Intern">{{ "intern"|l10n(page_data.lang) }}</option> | ||
| <option value="Fellow">{{ "fellow"|l10n(page_data.lang) }}</option> | ||
| <option value="Mod">{{ "mod"|l10n(page_data.lang) }}</option> | ||
| <option value="Super">{{ "super"|l10n(page_data.lang) }}</option> |
There was a problem hiding this comment.
Critical: Value type mismatch may cause panics
The changes introduce string values ("Pending", "Rejected", etc.) for roles, but the codebase appears to use numeric values (1=pending, 2=rejected, etc.) as seen in user_list.html. This type mismatch could be causing the panics.
Modify the values to match the numeric system:
- <option selected value="Pending">{{ "pending"|l10n(page_data.lang) }}</option>
- <option value="Rejected">{{ "rejected"|l10n(page_data.lang) }}</option>
- <option value="Limited">{{ "limited"|l10n(page_data.lang) }}</option>
- <option value="Intern">{{ "intern"|l10n(page_data.lang) }}</option>
- <option value="Fellow">{{ "fellow"|l10n(page_data.lang) }}</option>
- <option value="Mod">{{ "mod"|l10n(page_data.lang) }}</option>
- <option value="Super">{{ "super"|l10n(page_data.lang) }}</option>
+ <option selected value="1">{{ "pending"|l10n(page_data.lang) }}</option>
+ <option value="2">{{ "rejected"|l10n(page_data.lang) }}</option>
+ <option value="3">{{ "limited"|l10n(page_data.lang) }}</option>
+ <option value="4">{{ "intern"|l10n(page_data.lang) }}</option>
+ <option value="5">{{ "fellow"|l10n(page_data.lang) }}</option>
+ <option value="7">{{ "mod"|l10n(page_data.lang) }}</option>
+ <option value="10">{{ "super"|l10n(page_data.lang) }}</option>📝 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.
| <option selected value="Pending">{{ "pending"|l10n(page_data.lang) }}</option> | |
| <option value="Rejected">{{ "rejected"|l10n(page_data.lang) }}</option> | |
| <option value="Limited">{{ "limited"|l10n(page_data.lang) }}</option> | |
| <option value="Intern">{{ "intern"|l10n(page_data.lang) }}</option> | |
| <option value="Fellow">{{ "fellow"|l10n(page_data.lang) }}</option> | |
| <option value="Mod">{{ "mod"|l10n(page_data.lang) }}</option> | |
| <option value="Super">{{ "super"|l10n(page_data.lang) }}</option> | |
| <option selected value="1">{{ "pending"|l10n(page_data.lang) }}</option> | |
| <option value="2">{{ "rejected"|l10n(page_data.lang) }}</option> | |
| <option value="3">{{ "limited"|l10n(page_data.lang) }}</option> | |
| <option value="4">{{ "intern"|l10n(page_data.lang) }}</option> | |
| <option value="5">{{ "fellow"|l10n(page_data.lang) }}</option> | |
| <option value="7">{{ "mod"|l10n(page_data.lang) }}</option> | |
| <option value="10">{{ "super"|l10n(page_data.lang) }}</option> |
| <option value="Admin" {% if user.role == 255 %} selected {% endif %}>{{ "admin"|l10n(page_data.lang) }}</option> | ||
| <option value="Senior" {% if user.role == 100 %} selected {% endif %}>{{ "senior"|l10n(page_data.lang) }}</option> | ||
| <option value="Standard" {% if user.role == 10 %} selected {% endif %}>{{ "standard"|l10n(page_data.lang) }}</option> | ||
| <option value="Banned" {% if user.role == 0 %} selected {% endif %}>{{ "banned"|l10n(page_data.lang) }}</option> |
There was a problem hiding this comment.
Critical: Global role values must be numeric to match backend expectations
Similar to the Inn roles, the global role values should be numeric to match the backend's expectations.
Update the values to use numbers:
- <option value="Admin" {% if user.role == 255 %} selected {% endif %}>{{ "admin"|l10n(page_data.lang) }}</option>
- <option value="Senior" {% if user.role == 100 %} selected {% endif %}>{{ "senior"|l10n(page_data.lang) }}</option>
- <option value="Standard" {% if user.role == 10 %} selected {% endif %}>{{ "standard"|l10n(page_data.lang) }}</option>
- <option value="Banned" {% if user.role == 0 %} selected {% endif %}>{{ "banned"|l10n(page_data.lang) }}</option>
+ <option value="255" {% if user.role == 255 %} selected {% endif %}>{{ "admin"|l10n(page_data.lang) }}</option>
+ <option value="100" {% if user.role == 100 %} selected {% endif %}>{{ "senior"|l10n(page_data.lang) }}</option>
+ <option value="10" {% if user.role == 10 %} selected {% endif %}>{{ "standard"|l10n(page_data.lang) }}</option>
+ <option value="0" {% if user.role == 0 %} selected {% endif %}>{{ "banned"|l10n(page_data.lang) }}</option>📝 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.
| <option value="Admin" {% if user.role == 255 %} selected {% endif %}>{{ "admin"|l10n(page_data.lang) }}</option> | |
| <option value="Senior" {% if user.role == 100 %} selected {% endif %}>{{ "senior"|l10n(page_data.lang) }}</option> | |
| <option value="Standard" {% if user.role == 10 %} selected {% endif %}>{{ "standard"|l10n(page_data.lang) }}</option> | |
| <option value="Banned" {% if user.role == 0 %} selected {% endif %}>{{ "banned"|l10n(page_data.lang) }}</option> | |
| <option value="255" {% if user.role == 255 %} selected {% endif %}>{{ "admin"|l10n(page_data.lang) }}</option> | |
| <option value="100" {% if user.role == 100 %} selected {% endif %}>{{ "senior"|l10n(page_data.lang) }}</option> | |
| <option value="10" {% if user.role == 10 %} selected {% endif %}>{{ "standard"|l10n(page_data.lang) }}</option> | |
| <option value="0" {% if user.role == 0 %} selected {% endif %}>{{ "banned"|l10n(page_data.lang) }}</option> |
| <option value="Pending" {% if user.role == 1 %} selected {% endif %}>{{ "pending"|l10n(page_data.lang) }}</option> | ||
| <option value="Rejected" {% if user.role == 2 %} selected {% endif %}>{{ "rejected"|l10n(page_data.lang) }}</option> | ||
| {% endif %} | ||
| <option {% if user.role == 3 %} selected {% endif %}>{{ "limited"|l10n(page_data.lang) }}</option> | ||
| <option {% if user.role == 4 %} selected {% endif %}>{{ "intern"|l10n(page_data.lang) }}</option> | ||
| <option {% if user.role == 5 %} selected {% endif %}>{{ "fellow"|l10n(page_data.lang) }}</option> | ||
| <option {% if user.role == 7 %} selected {% endif %}>{{ "mod"|l10n(page_data.lang) }}</option> | ||
| <option {% if user.role == 10 %} selected {% endif %}>{{ "super"|l10n(page_data.lang) }}</option> | ||
| <option value="Limited" {% if user.role == 3 %} selected {% endif %}>{{ "limited"|l10n(page_data.lang) }}</option> | ||
| <option value="Intern" {% if user.role == 4 %} selected {% endif %}>{{ "intern"|l10n(page_data.lang) }}</option> | ||
| <option value="Fellow" {% if user.role == 5 %} selected {% endif %}>{{ "fellow"|l10n(page_data.lang) }}</option> | ||
| <option value="Mod" {% if user.role == 7 %} selected {% endif %}>{{ "mod"|l10n(page_data.lang) }}</option> | ||
| <option value="Super" {% if user.role == 10 %} selected {% endif %}>{{ "super"|l10n(page_data.lang) }}</option> |
There was a problem hiding this comment.
Critical: Inn role values must be numeric to match backend expectations
The template compares roles using numeric values (e.g., {% if user.role == 1 %}), but the select options use string values. This mismatch could cause panics when processing form submissions.
Update the values to use numbers:
- <option value="Pending" {% if user.role == 1 %} selected {% endif %}>{{ "pending"|l10n(page_data.lang) }}</option>
- <option value="Rejected" {% if user.role == 2 %} selected {% endif %}>{{ "rejected"|l10n(page_data.lang) }}</option>
- <option value="Limited" {% if user.role == 3 %} selected {% endif %}>{{ "limited"|l10n(page_data.lang) }}</option>
- <option value="Intern" {% if user.role == 4 %} selected {% endif %}>{{ "intern"|l10n(page_data.lang) }}</option>
- <option value="Fellow" {% if user.role == 5 %} selected {% endif %}>{{ "fellow"|l10n(page_data.lang) }}</option>
- <option value="Mod" {% if user.role == 7 %} selected {% endif %}>{{ "mod"|l10n(page_data.lang) }}</option>
- <option value="Super" {% if user.role == 10 %} selected {% endif %}>{{ "super"|l10n(page_data.lang) }}</option>
+ <option value="1" {% if user.role == 1 %} selected {% endif %}>{{ "pending"|l10n(page_data.lang) }}</option>
+ <option value="2" {% if user.role == 2 %} selected {% endif %}>{{ "rejected"|l10n(page_data.lang) }}</option>
+ <option value="3" {% if user.role == 3 %} selected {% endif %}>{{ "limited"|l10n(page_data.lang) }}</option>
+ <option value="4" {% if user.role == 4 %} selected {% endif %}>{{ "intern"|l10n(page_data.lang) }}</option>
+ <option value="5" {% if user.role == 5 %} selected {% endif %}>{{ "fellow"|l10n(page_data.lang) }}</option>
+ <option value="7" {% if user.role == 7 %} selected {% endif %}>{{ "mod"|l10n(page_data.lang) }}</option>
+ <option value="10" {% if user.role == 10 %} selected {% endif %}>{{ "super"|l10n(page_data.lang) }}</option>📝 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.
| <option value="Pending" {% if user.role == 1 %} selected {% endif %}>{{ "pending"|l10n(page_data.lang) }}</option> | |
| <option value="Rejected" {% if user.role == 2 %} selected {% endif %}>{{ "rejected"|l10n(page_data.lang) }}</option> | |
| {% endif %} | |
| <option {% if user.role == 3 %} selected {% endif %}>{{ "limited"|l10n(page_data.lang) }}</option> | |
| <option {% if user.role == 4 %} selected {% endif %}>{{ "intern"|l10n(page_data.lang) }}</option> | |
| <option {% if user.role == 5 %} selected {% endif %}>{{ "fellow"|l10n(page_data.lang) }}</option> | |
| <option {% if user.role == 7 %} selected {% endif %}>{{ "mod"|l10n(page_data.lang) }}</option> | |
| <option {% if user.role == 10 %} selected {% endif %}>{{ "super"|l10n(page_data.lang) }}</option> | |
| <option value="Limited" {% if user.role == 3 %} selected {% endif %}>{{ "limited"|l10n(page_data.lang) }}</option> | |
| <option value="Intern" {% if user.role == 4 %} selected {% endif %}>{{ "intern"|l10n(page_data.lang) }}</option> | |
| <option value="Fellow" {% if user.role == 5 %} selected {% endif %}>{{ "fellow"|l10n(page_data.lang) }}</option> | |
| <option value="Mod" {% if user.role == 7 %} selected {% endif %}>{{ "mod"|l10n(page_data.lang) }}</option> | |
| <option value="Super" {% if user.role == 10 %} selected {% endif %}>{{ "super"|l10n(page_data.lang) }}</option> | |
| <option value="1" {% if user.role == 1 %} selected {% endif %}>{{ "pending"|l10n(page_data.lang) }}</option> | |
| <option value="2" {% if user.role == 2 %} selected {% endif %}>{{ "rejected"|l10n(page_data.lang) }}</option> | |
| {% endif %} | |
| <option value="3" {% if user.role == 3 %} selected {% endif %}>{{ "limited"|l10n(page_data.lang) }}</option> | |
| <option value="4" {% if user.role == 4 %} selected {% endif %}>{{ "intern"|l10n(page_data.lang) }}</option> | |
| <option value="5" {% if user.role == 5 %} selected {% endif %}>{{ "fellow"|l10n(page_data.lang) }}</option> | |
| <option value="7" {% if user.role == 7 %} selected {% endif %}>{{ "mod"|l10n(page_data.lang) }}</option> | |
| <option value="10" {% if user.role == 10 %} selected {% endif %}>{{ "super"|l10n(page_data.lang) }}</option> |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/controller/meta_handler.rs (1)
254-266: Robust error handling in template rendering.
By returning aResult<Response, rinja::Error>and mapping to a custom error, you ensure template rendering failures won't crash the server. Consider including a trace-level log oferrif you’d like deeper diagnostics.src/controller/user.rs (2)
588-588:FormUser.usernamevalidation ensures length is bounded (1..=32).Consider also restricting to allowed characters if needed to prevent unexpected inputs.
596-596:FormUser.langalso skips validation.
If arbitrary language codes are allowed, this is suitable. Otherwise, consider enumerating valid codes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.toml(2 hunks)src/controller/admin.rs(3 hunks)src/controller/feed.rs(4 hunks)src/controller/inn.rs(7 hunks)src/controller/message.rs(1 hunks)src/controller/meta_handler.rs(2 hunks)src/controller/mod.rs(3 hunks)src/controller/notification.rs(2 hunks)src/controller/solo.rs(3 hunks)src/controller/tantivy.rs(2 hunks)src/controller/upload.rs(2 hunks)src/controller/user.rs(10 hunks)src/error.rs(1 hunks)
🔇 Additional comments (69)
src/controller/meta_handler.rs (4)
6-6: Imports consolidated with new modules.
These import statements align well with the transition tovalidator-based form validation and the usage ofrinja::Template. No issues spotted.Also applies to: 9-9, 16-16, 19-19
228-229: Struct definition looks good.
ValidatedForm<T>encapsulates validated form data succinctly. No concerns regarding memory footprint or struct usage at this point.
232-245: Well-structured form validation approach.
ImplementingFromRequestto validate incoming form data improves maintainability and ensures consistent error handling. This looks robust.
247-253: Graceful template rendering function.
Wrapping thetry_into_responsecall in standard Axum error conversions keeps response generation uniform and clear.src/error.rs (1)
49-49: Seamless migration tovalidator::ValidationErrors.
Switching fromgarde::Errortovalidator::ValidationErrorsmaintains clarity and consistency with the new validation scheme.src/controller/message.rs (2)
7-7: Template import realigned.
Moving torinja::Templateis coherent with the rest of the refactoring.
15-15: Consistent usage ofinto_response.
Importinginto_responsefrommeta_handlerensures that response construction follows the same pattern used throughout the codebase.src/controller/upload.rs (2)
5-5: New response utilities integrated.
Incorporatinginto_responseandget_refererfrommeta_handlerunifies the approach to generating responses and retrieving referer headers.
25-25: Refined import path for the template engine.
Usingrinja::Templateis consistent with the broader switch away fromrinja_axum.src/controller/tantivy.rs (2)
8-8: Applying the new templating source.
No issues—import logic is consolidated consistently, supporting the same approach found elsewhere.
37-37: Unified response handling.
Integratinginto_responsefrommeta_handlercontinues the standardized pattern across controllers.src/controller/notification.rs (2)
3-3: Prefer consistent import style with other controllers
The change importsinto_responseandPageDatafrommeta_handler, aligning with similar refactors in other controllers. This standardization looks good and helps keep response handling uniform across the project.
12-12: Template import usage
Importingrinja::Templatehere indicates that notification pages are using Rinja templates, which matches the approach seen in other parts of the codebase.src/controller/mod.rs (3)
146-146: Newly introduced validator dependency
Good job transitioning tovalidator::Validate. This change ensures consistency across files using the same validation library.
389-399: Validation attributes for FormPost
All these attributes correctly switch fromgardetovalidatornotations. The min/max constraints and skip options look consistent with the expected logic. Ensure that all form handlers usingFormPostare updated accordingly.
454-487: Validation attributes in SiteConfig
The new validator attributes for each field match the original constraints, preserving data consistency (e.g.,length(max = 64)andrange(max = X)). Ensure thorough testing of each constraint, especially around boundary conditions.src/controller/admin.rs (4)
5-5: Import refactor
ReplacingWithValidationwithValidatedFormclarifies how form validation is integrated into your handlers. This aligns the admin controller with the broader migration away fromgarde.
20-20: Confirm template usage
The direct usage ofrinja::Templateis consistent with the approach across other controllers. No issues here.
351-351: Use of ValidatedForm for site settings
Switching the admin site settings handler toValidatedForm<SiteConfig>removes the extra layer of calls and keeps logic straightforward. Great job.
359-359: Sanitization step
Storinginputinsite_configand immediately sanitizing user-provided fields (e.g.,site_name,domain) is a good practice. This helps mitigate HTML injection.src/controller/solo.rs (5)
8-8: Centralized response handling
Importinginto_response,PageData, andValidatedFormfrommeta_handleraligns the solo controller with the rest of the codebase.
24-24: Template engine utilization
No issues with including the template forsolo. Consistent with other modules in the codebase.
28-28: Validator usage
Similar to other controllers, adoptingvalidator::Validateis consistent and ensures a cohesive validation strategy.
33-37: FormSolo validation attributes
Switching from#[garde(...)]to#[validate(...)]while maintaining the same constraints (e.g.,length(min = 1, max = 1000)for content) looks correct.
382-382: ValidatedForm adoption
ReplacingWithValidation<Form<FormSolo>>withValidatedForm<FormSolo>streamlines the code, reducing boilerplate in request handling and making the logic clearer.src/controller/feed.rs (4)
8-8: Use ofinto_responseis consistent with the PR’s validation pattern.
29-29: Importingrinja::Templateseamlessly supports Tera-based templating.
38-38: Switch tovalidator::Validatealigns with the library transition plan.
480-486:FormFeedAdd: Updated field-level validations.
The#[validate(...)]attributes here look correct and well-aligned with the newvalidatorlibrary's syntax.src/controller/user.rs (19)
11-11: AdoptingValidatedFormfrommeta_handlerensures standardized form validation.
35-35: Importingrinja::Templatefor Tera-based rendering is consistent with existing usage.
40-40: Switch tovalidator::Validateis consistent with the PR’s changes.
590-590:FormUser.aboutuses a maximum length of 1024.
592-592:FormUser.urluses a maximum length of 256.
594-594:FormUser.home_pageuses#[validate(skip)].
Having this field skip validation is reasonable if the logic is handled differently.
685-685:FormReset.password2enforces a minimum length of 7.
694-696: Password mismatch error.
Returning a clear “Passwords do not match” message is user-friendly and correct.
740-740: Updated signature foruser_setting_postto utilizeValidatedForm.
791-791:FormPassword.old_passworduses#[validate(skip)].
No immediate concerns; old password typically can’t be validated for length alone.
794-794:FormPassword.password2has a minimum length of 7.
801-801:user_password_postusesValidatedForm<FormPassword>to ensure correctness.
803-805: Password mismatch check inuser_password_post.
Delays reattempt withsleepis a good measure against brute force.
888-888:FormSignup.usernamemandates 1..=32 length.
891-891:FormSignup.password2set to minimum length of 7.
963-963:signup_postusesValidatedForm<FormSignup>.
Good approach to unify validation.
965-967: Password mismatch error insignup_post.
Ensures user clarity on invalid repeated password.
1049-1049:FormRecoverySet.passwordset to minimum length of 7.
1056-1056:user_recovery_codenow usesValidatedForm<FormRecoverySet>.
Shortens code and clarifies error handling.src/controller/inn.rs (17)
23-23: Import ofValidatedFormis consistent with the new validation model.
39-39:use rinja::import indicates Tera templating usage.
40-40: Additional filter imports (escape,Html) for sanitizing user data.
41-41:use rinja::Templatefor Tera-based template rendering.
42-42: Scope closure of therinjaimports.
47-47:use validator::Validatemarks the formal adoption of the new library.
119-119:FormInn.inn_nameenforces minimum=1, maximum=64.
121-121:FormInn.aboutenforces length within [1..=512].
123-123:FormInn.descriptioncontent bounded within [1..=65535].
125-125:FormInn.topicslength within [1..=128].
127-127:FormInn.inn_typeuses#[validate(skip)].
This is likely handled by specialized logic.
129-129:FormInn.early_birdsuses#[validate(skip)].
Potentially used for an advanced logic or defaults.
131-131:FormInn.limit_edit_secondsuses#[validate(skip)].
Big or zero values can be valid for unlimited editing.
141-141:mod_inn_post: switch fromWithValidationtoValidatedForm<FormInn>.
1749-1749:FormComment.contentlimited to [1..=10000].
1757-1757:comment_post: use ofValidatedForm<FormComment>.
1898-1898:previewmethod usesValidatedForm<FormComment>.
Helps unify validation across code paths.Cargo.toml (4)
10-10: Upgraded axum to 0.8.1Ensure feature compatibility with your existing code. Verify major changes or deprecations from older versions.
11-11: Updatedaxum-extrato 0.10.0Check for any breaking changes in typed header usage or subtle interface changes.
24-24: Upgradedjiffto 0.1.18
49-49: New dependency:validatorv0.19.0
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app_router.rs (1)
111-111: Message route changes
/message/{uid}has replaced the colon syntax. Make sure this is also updated in any JavaScript or template code that references the old route.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
Cargo.toml(3 hunks)src/app_router.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Cargo.toml
🔇 Additional comments (9)
src/app_router.rs (9)
70-72: Image deletion and moderation routes updated.
The changes to{uid},{img_id}, and{iid}maintain consistency. Ensure the handlers properly parse these IDs (likely integers).
95-98: Solo routes likewise updated.
The transitions from:uto{u},:sidto{sid}are correct. Confirm the views and controllers are updated to use{sid}in path building.
103-103: Gallery route uses new parameter notation.
Straightforward update from:uidto{uid}. Ensure any function calls or references consistently use{uid}.
107-109: Feed routes star, subscribe, and read now match updated style.
The route definitions with{item_id}and{uid}reflect best practices for Axum path parameters. No immediate concerns.
113-113: Inbox route parameter
Replacing:midwith{mid}is consistent. Looks good, and no collisions are apparent.
64-65: Updated session removal and role assignment routes.
Both route definitions align with the new curly brace syntax. Double-check that thesession_idandid+uidparameters are handled consistently in the related handlers, especially any data type conversions.✅ Verification successful
Route parameters are correctly aligned with handler function signatures
The verification confirms that the route parameters and handler functions are properly aligned:
/user/remove/{session_id}route:
- Handler
remove_sessionacceptsPath(session_id): Path<String>/role/{id}/{uid}route:
- Handler
role_postacceptsPath((id, uid)): Path<(u32, u32)>The parameter types are properly handled with appropriate type conversions - String for session_id and u32 for both id and uid parameters.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of `remove_session` and `role_post` to confirm parameter alignment rg -A 3 "remove_session|role_post"Length of output: 1221
77-79: Post locking, hiding, pinning now use consistent parameter syntax.
Replacing the colon prefix with curly braces is correct for newer Axum versions. Verify the handlers for any mismatched argument names.✅ Verification successful
Parameter names in route handlers match the path parameters
The verification confirms that the parameter names in the handler functions match the path parameters correctly:
- In
src/app_router.rs, the routes use{iid}and{pid}parameters- In
src/controller/inn.rs, all three handlers (post_lock,post_hide,post_pin) use matchingPath((iid, pid))parameters in their signatures🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check relevant mod_inn or post_* function signatures to confirm parameter alignment rg -A 3 "fn .*post_lock|post_hide|post_pin"Length of output: 2975
81-93: Inn and post routes adopt curly brace parameters.
This large block ensures uniform usage of{tag},{i},{iid},{pid}, and{cid}. Confirm that all future references in templates or forms are updated accordingly, especially if these routes were previously accessed under older parameters.✅ Verification successful
Let me search for template references to verify if there are any inconsistencies.
All route parameter references are consistent with the new curly brace format
The verification shows that all references to inn and post routes in the codebase are using the curly brace parameter format consistently. The matches found in the last search are not problematic as they are either:
- References to static routes (e.g.,
/inn/0,/inn/list)- Template variables that get properly interpolated (e.g.,
/inn/{{iid}},/post/{{post.iid}}/{{post.pid}})Both the route definitions in
app_router.rsand their usage in templates and controllers maintain a consistent parameter format using curly braces.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify references to inn and post route patterns rg -A 3 "/inn/\{|/post/\{"Length of output: 21488
Script:
#!/bin/bash # Search for any old parameter patterns in templates rg -g '*.html' '/inn/[^{]|/post/[^{]'Length of output: 7920
56-57: Consistent route parameter syntax.
These routes properly adopt the curly brace syntax for path parameters. Make sure to verify that downstream references in your controllers and views useuconsistently.✅ Verification successful
Let me search for the handler function implementations and parameter usage to verify the consistency of the path parameter.
Let me try different patterns to find the handler implementations and parameter usage.
Path parameter 'u' is consistently used in handler functions
The verification shows that both route handlers
useranduser_followconsistently usePath(u): Path<String>to extract the path parameter 'u', which matches the route definitions inapp_router.rs. The parameter naming and extraction method are uniform across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check references to the path parameter "u" throughout the codebase rg -A 4 "fn .*user\(.*u.*\)"Length of output: 30
Script:
#!/bin/bash # Search for function definitions related to user routes ast-grep --pattern 'fn user($$$) { $$$ }' # Also check for user_follow function ast-grep --pattern 'fn user_follow($$$) { $$$ }' # Search for any usage of path parameter "u" rg -A 2 'Path::param.*"u"'Length of output: 127
Script:
#!/bin/bash # Search for handler functions in a more general way rg -A 5 "async fn user" # Search for path parameter extraction using different patterns rg -A 3 "params.*[\"']u[\"']" rg -A 3 "extract.*[\"']u[\"']"Length of output: 3017
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/controller/user.rs (1)
791-795: Consider strengthening password requirements.While the current minimum password length of 7 characters provides basic security, consider:
- Increasing minimum length to 8 characters (NIST recommendation)
- Adding additional validation rules for password complexity
- #[validate(length(min = 7))] + #[validate(length(min = 8))] password2: String,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/controller/upload.rs(3 hunks)src/controller/user.rs(13 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/controller/upload.rs
🔇 Additional comments (5)
src/controller/user.rs (5)
531-531: Good improvement in error handling!Replacing
unreachable!()with proper error handling usingAppError::Unauthorizedis a significant improvement. This change prevents potential panics in production and provides better error feedback.Also applies to: 567-567
588-596: Well-defined validation constraints!The validation rules for the FormUser struct are well-thought-out with appropriate length constraints for each field:
- Username length (1-32 chars) is reasonable for user identification
- About text length (max 1024 chars) provides enough space for user bios
- URL length (max 256 chars) is sufficient for most web URLs
888-892: Align password validation with security best practices.The password validation here should be updated to match the suggested improvements in the FormPassword struct (minimum 8 characters).
1049-1051: Standardize password validation across all forms.Update the password validation to match the suggested improvements in other password-related forms.
Line range hint
1132-1146: Secure password hashing implementation!The password hashing implementation follows cryptographic best practices:
- Uses PBKDF2-HMAC-SHA512 with 100,000 iterations
- Generates cryptographically secure random salt using SystemRandom
- Uses sufficient salt size (64 bytes)
| if input.password != input.password2 { | ||
| return Err(AppError::Custom("Passwords do not match".to_string())); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor duplicated password validation logic.
The password matching validation logic is duplicated across three functions. Consider extracting this into a shared validation function to improve maintainability and consistency.
fn validate_matching_passwords(password: &str, password2: &str) -> Result<(), AppError> {
if password != password2 {
return Err(AppError::Custom("Passwords do not match".to_string()));
}
Ok(())
}Also applies to: 803-805, 965-967
| @@ -915,15 +915,15 @@ | |||
| "Easy" => Difficulty::Easy, | |||
| "Medium" => Difficulty::Medium, | |||
| "Hard" => Difficulty::Hard, | |||
| _ => unreachable!(), | |||
| _ => return Err(AppError::NotFound), | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add explicit error handling for invalid captcha configurations.
Similar to the role validation improvement, replace the implicit unreachable!() with proper error handling for invalid captcha configurations.
- _ => return Err(AppError::NotFound),
+ invalid => return Err(AppError::Custom(format!("Invalid captcha difficulty: {}", invalid))),Also applies to: 926-926
Summary by CodeRabbit
valueattributes to role selection options.gardetovalidatorlibrary for form validation, updating relevant attributes and imports throughout the application.