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 Spaces in passwords are not handled correctly when adding servers #822

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

Shadow243
Copy link
Member

Issue Description

I encountered an issue where spaces in passwords were being encoded as + when submitting a POST request with the Content-Type application/x-www-form-urlencoded. This behavior was causing problems, as passwords with spaces were not processed correctly on the server.

Proposed Correction

To address this issue, I added URL decoding (urldecode()) to the server-side code to properly decode data received from the POST request. This ensures that spaces in passwords are correctly handled and not transformed into + symbols. As a result, passwords containing spaces are now processed as expected.

Steps to Reproduce

  1. Submit a POST request with Content-Type application/x-www-form-urlencoded.
  2. Include a password with spaces in the request data.
  3. Observe that the spaces are correctly decoded on the server side.

This change should resolve the issue as described in Issue #821 and ensure that passwords with spaces are handled correctly in POST requests.

@kroky
Copy link
Member

kroky commented Dec 1, 2023

@Shadow243 the problem is not the content type as PHP handles content types correctly by decoding incoming encoded types in POST, GET, etc. arrays. The problem should be the FILTER_UNSAFE_RAW filter applied on imap_pass field specified in modules/imap/setup.php . That filter seems to encode special characters. We don't want any encoding or changes in password fields - just ignore anything as a filter - we need the password as user submitted it.

@Shadow243
Copy link
Member Author

Copy that, I'm checking it out...

@Shadow243
Copy link
Member Author

After conducting checks by modifying or removing the filter, it seems that the issue might originate from the encoding done at line 182 of the modules/core/site.js file. The line xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded'); specifies an encoding that replaces non-alphanumeric characters, such as spaces, with the "+" symbol.

By adjusting the content type from "application/x-www-form-urlencoded" to "multipart/form-data," the data is no longer encoded. However, this change alters the structure in Cypht, bringing us back to the previously suggested solution if preserving the existing structure is desired.

The comment also references a list of tested filters, sourced from filter.constants.php and filter.filters.sanitize.php. After conducting thorough tests, it has become evident that the undesired transformation is indeed linked to the "application/x-www-form-urlencoded" encoding.

Additional Information

@kroky
Copy link
Member

kroky commented Dec 4, 2023

@Shadow243 can you try submitting a normal browser form element with application/x-www-form-urlencoded enctype and see if the variable on the cypht backend is still badly encoded? If so, we should definitely fix site.js encoding function format_xhr_data, so it works with PHP decoding system.

@Shadow243
Copy link
Member Author

@Shadow243 can you try submitting a normal browser form element with application/x-www-form-urlencoded enctype and see if the variable on the cypht backend is still badly encoded? If so, we should definitely fix site.js encoding function format_xhr_data, so it works with PHP decoding system.

Trying this one, not getting transformed data with +.

An alternative solution from: https://itecnote.com/tecnote/jquery-serialize-converts-all-spaces-to-plus/ is working.

Changes can be seen here: #822

modules/core/site.js Outdated Show resolved Hide resolved
@Shadow243 Shadow243 force-pushed the smtp-imap-space-in-password branch 2 times, most recently from d5e32d1 to 2509118 Compare March 5, 2024 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants