Fix invalid URI Encoding in API request handling

[delano]

User description

The changes introduce a new middleware Rack::HandleInvalidUTF8 to handle invalid UTF-8 characters in HTTP requests. This middleware checks various parts of the request environment for invalid UTF-8 sequences and raises an exception if any are found, returning a 400 Bad Request response. Additionally, the existing Rack::HandleInvalidPercentEncoding middleware is enhanced to handle exceptions more gracefully and log errors.

Key changes include:

• Addition of lib/middleware/handle_invalid_utf8.rb to define the new middleware.
• Extensive tests in try/07_handle_invalid_utf8_try.rb to validate the new middleware's functionality.
• Enhancements to lib/middleware/handle_invalid_percent_encoding.rb to improve error handling and logging.
• Introduction of lib/onetime/app/base.rb to provide configuration options for UTF-8 and URI encoding checks.
• Updates to config.ru to include the new middleware in the middleware stack.
• Addition of inline descriptions in tryout files.
• Modifications to lib/onetime/app/api.rb and lib/onetime/app/api/base.rb to include the new settings and improve relative path requires.
• Enabling Performance/Size cop in .rubocop.yml for better code quality.

---

PR Type

Enhancement, Tests, Documentation

---

Description

• Added Rack::HandleInvalidUTF8 middleware to handle invalid UTF-8 characters in HTTP requests.
• Enhanced Rack::HandleInvalidPercentEncoding middleware for better error handling and logging.
• Introduced AppSettings module for configuring UTF-8 and URI encoding checks.
• Updated tests to cover new middleware functionalities and configurations.
• Added detailed descriptions to various tryout files for better documentation.
• Enabled Performance/Size cop in .rubocop.yml for improved code quality.

---

Changes walkthrough 📝

	Relevant files
Enhancement	7 files config.ru Add UTF-8 handling middleware and improve logging                config.ru Added Rack::HandleInvalidUTF8 middleware to handle invalid UTF-8 characters. Ensured immediate flushing of stdout for real-time logging. +7/-0      handle_invalid_percent_encoding.rb Improve error handling and logging in percent encoding middleware lib/middleware/handle_invalid_percent_encoding.rb Enhanced to handle exceptions more gracefully. Added logging improvements. Introduced check_enabled? method for route-specific checks. +48/-18  handle_invalid_utf8.rb Add middleware to handle invalid UTF-8 characters                lib/middleware/handle_invalid_utf8.rb Introduced new middleware to handle invalid UTF-8 characters. Added logging and error handling for invalid UTF-8 sequences. Implemented route-specific checks for UTF-8 validation. +143/-0  api.rb Integrate AppSettings module for encoding checks                  lib/onetime/app/api.rb Included AppSettings module for UTF-8 and URI encoding checks. Set default values for check_utf8 and check_uri_encoding. +5/-0      base.rb Update relative path requires in API base                                lib/onetime/app/api/base.rb Updated relative path requires for better maintainability. +3/-1      base.rb Add AppSettings module for encoding middleware configuration lib/onetime/app/base.rb Added AppSettings module for configuration of encoding checks. Provided class-level accessors for check_utf8 and check_uri_encoding. +33/-0    base.rb Update relative path requires in web base                                lib/onetime/app/web/base.rb Updated relative path requires for better maintainability. +1/-1
Tests	3 files 06_handle_invalid_percent_encoding_try.rb Update tests for percent encoding middleware                          try/06_handle_invalid_percent_encoding_try.rb Updated tests to reflect changes in percent encoding middleware. Ensured middleware sets correct content type in error responses. +9/-9      07_handle_invalid_utf8_try.rb Add tests for UTF-8 handling middleware                                    try/07_handle_invalid_utf8_try.rb Added tests for the new UTF-8 handling middleware. Covered various scenarios including invalid UTF-8 in headers and body. +117/-0  23_app_settings_try.rb Add tests for AppSettings module                                                  try/23_app_settings_try.rb Added tests for AppSettings module. Covered default values and setting/getting configurations. +73/-0
Documentation	16 files 10_utils_try.rb Document utility function tests                                                    try/10_utils_try.rb Added detailed descriptions for utility function tests. +16/-0    11_entropy_try.rb Document entropy module tests                                                        try/11_entropy_try.rb Added detailed descriptions for entropy module tests. +13/-0    15_config_try.rb Document configuration tests                                                          try/15_config_try.rb Added detailed descriptions for configuration tests. +12/-0    17_mail_validation.rb Document email validation tests                                                    try/17_mail_validation.rb Added detailed descriptions for email validation tests. +14/-1    20_metadata_try.rb Document metadata class tests                                                        try/20_metadata_try.rb Added detailed descriptions for metadata class tests. +14/-0    21_secret_try.rb Document secret class tests                                                            try/21_secret_try.rb Added detailed descriptions for secret class tests. +13/-0    22_value_encryption_try.rb Document value encryption tests                                                    try/22_value_encryption_try.rb Added detailed descriptions for value encryption tests. +18/-0    23_passphrase_try.rb Document passphrase handling tests                                              try/23_passphrase_try.rb Added detailed descriptions for passphrase handling tests. +12/-0    25_customer_try.rb Document customer model tests                                                        try/25_customer_try.rb Added detailed descriptions for customer model tests. +17/-0    30_session_try.rb Document session management tests                                                try/30_session_try.rb Added detailed descriptions for session management tests. +17/-0    35_ratelimit_try.rb Document rate limiting tests                                                          try/35_ratelimit_try.rb Added detailed descriptions for rate limiting tests. +16/-0    40_email_template_try.rb Document email template tests                                                        try/40_email_template_try.rb Added detailed descriptions for email template tests. +13/-0    50_subdomain_try.rb Document subdomain functionality tests                                      try/50_subdomain_try.rb Added detailed descriptions for subdomain functionality tests. +16/-0    60_logic_base_try.rb Document base logic tests                                                                try/60_logic_base_try.rb Added detailed descriptions for base logic tests. +16/-0    61_logic_destroy_account_try.rb Document account destruction tests                                              try/61_logic_destroy_account_try.rb Added detailed descriptions for account destruction tests. +15/-0    68_receive_feedback_try.rb Document feedback receiving tests                                                try/68_receive_feedback_try.rb Added detailed descriptions for feedback receiving tests. +15/-0
Configuration changes	1 files .rubocop.yml Enable Performance/Size cop in RuboCop configuration          .rubocop.yml Enabled Performance/Size cop for better code quality. +1/-0

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Code Duplication The method handle_exception in handle_invalid_percent_encoding.rb and handle_invalid_utf8.rb seems to have duplicated logic for handling exceptions. Consider refactoring this into a shared module or superclass to reduce duplication and improve maintainability. Debugging Code The file contains a require 'pry-byebug' statement, which is typically used for debugging. This should not be included in production code as it can lead to performance issues and potentially expose sensitive information. Exception Handling The method check_and_raise_for_invalid_utf8 raises a generic StandardError for invalid UTF-8 input. It would be better to define a custom exception class for clarity and to allow more precise rescue statements elsewhere in the application. Middleware Configuration The middleware Rack::HandleInvalidUTF8 is added globally in the config.ru file without any environment-specific checks. This might not be necessary for all environments (e.g., testing) and could lead to unexpected behavior or performance issues in those contexts.

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Thread safety	Use thread-local storage for variables that might be modified in a multi-threaded context It's recommended to use instance variables with caution in a multi-threaded environment like a web server. Consider using thread-local variables or other thread-safe mechanisms if these variables (@check_utf8, @check_uri_encoding) are meant to be modified at runtime. lib/onetime/app/api.rb [10-11] -@check_utf8 = true -@check_uri_encoding = true +Thread.current[:check_utf8] = true +Thread.current[:check_uri_encoding] = true Apply this suggestion Suggestion importance[1-10]: 9 Why: Using thread-local variables enhances thread safety, which is crucial in a multi-threaded environment like a web server. This suggestion addresses a significant potential issue.	9
Performance	Optimize the environment handling to avoid unnecessary duplication Instead of duplicating the entire environment, consider using a more efficient method to check for invalid UTF-8 without altering the original env. This can improve performance, especially for large environments. lib/middleware/handle_invalid_utf8.rb [61-65] -shallow_copy = env.dup self.class.input_sources.each do |key| - next unless shallow_copy.key?(key) - check_and_raise_for_invalid_utf8(shallow_copy[key], key) + next unless env.key?(key) + check_and_raise_for_invalid_utf8(env[key], key) Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion correctly identifies an opportunity to improve performance by avoiding unnecessary duplication of the environment. This change is efficient and maintains the functionality.	8
Best practice	✅ Change from require_relative to require for external gems to avoid path issues Suggestion Impact: The suggestion to use `require` instead of `require_relative` for the 'pry-byebug' gem was implemented, as the line with `require 'pry-byebug'` was removed. code diff: require_relative '../lib/onetime' -require 'pry-byebug'; Consider using require instead of require_relative for loading the 'pry-byebug' gem. This is because require is typically used for external libraries and gems, and it can help avoid issues related to relative paths in different environments. try/30_session_try.rb [21] -require 'pry-byebug'; +require 'pry-byebug' Apply this suggestion Suggestion importance[1-10]: 8 Why: Using require for external gems is a best practice to avoid path issues, especially in different environments. This suggestion improves code reliability.	8
Enhancement	✅ Enhance logging details for better issue tracking Improve the logging detail by including more context about the error and the affected part of the request. This can help in quicker identification and resolution of issues. lib/middleware/handle_invalid_utf8.rb [130] -logger.info "[handle-invalid-utf8] #{message}" +logger.info "[handle-invalid-utf8] Error occurred: #{message}. Affected URI: #{env['REQUEST_URI']}" [Suggestion has been applied] Suggestion importance[1-10]: 7 Why: The suggestion improves the logging detail by including more context about the error and the affected part of the request, which can aid in quicker identification and resolution of issues. This is a useful enhancement for debugging.	7
Maintainability	Add comments for clarity and maintainability in RuboCop configuration It's good practice to keep the RuboCop configuration organized and maintain a consistent structure. Consider adding a comment above the new Performance/Size setting to describe its purpose or the reason for enabling it, similar to other settings in the file. .rubocop.yml [66] +# Enable Performance/Size to ensure efficient code Enabled: true Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding comments improves the maintainability and clarity of the configuration file, making it easier for future developers to understand the purpose of settings.	7
	✅ Check for potential method conflicts after including helpers in API base Since the require_relative '../helpers' is added, ensure that any methods or modules included from Onetime::App::Helpers are not causing namespace conflicts or overriding methods unintentionally in Onetime::App::API::Base. lib/onetime/app/api/base.rb [1] -require_relative '../helpers' +require_relative '../helpers' # Ensure no conflicts with Onetime::App::API::Base methods [Suggestion has been applied] Suggestion importance[1-10]: 5 Why: While the suggestion to check for method conflicts is valid, it doesn't provide a concrete improvement to the code. It is more of a reminder for the developer.	5

[delano]

👌