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

Case-insensitive webfinger response. Fixes #1955 & #1986 #2005

Merged
merged 5 commits into from
Dec 20, 2021
Merged

Case-insensitive webfinger response. Fixes #1955 & #1986 #2005

merged 5 commits into from
Dec 20, 2021

Conversation

Kradyz
Copy link
Contributor

@Kradyz Kradyz commented Dec 19, 2021

Case-insensitive webfinger response

  • The webfinger regex in crates/utils/src/settings/mod.rs has been modified so that the server will no longer reject requests containing A-Z characters.

Case-insensitive database search

  • The sql_function “lower” has been added to crates/db_schema/src/lib.rs. This function allows one to perform a database searches applying lower(column) operations with diesel.

  • In /crates/db_schema/Cargo.toml, the [lib] block has been updated to be able to import the lib.rs functions.

  • The lower function is loaded by lemmy/crates/db_schema/src/impls/person.rs and lemmy/crates/db_schema/src/impls/community.rs, and used to perform a case-insensitive database searches via find_by_name and read_from_name, respectively. This allows the webfinger response to be case insensitive.

Include uppercase characters in the API_tests

  • The randomString generation function in api_tests/src/shared.ts has been updated to include uppercase characters when running tests.

Field tests

The user Sal can now be viewed in lemmy.ml:
https://lemmy.ml/u/Sal@mander.xyz

The following curls will trigger the correct webfinger responses for the user Sal and for the community main.

curl "https://mander.xyz/.well-known/webfinger?resource=acct:SaL@mander.xyz"
curl "https://mander.xyz/.well-known/webfinger?resource=acct:sal@mander.xyz"

curl "https://mander.xyz/.well-known/webfinger?resource=acct:main@mander.xyz"
curl "https://mander.xyz/.well-known/webfinger?resource=acct:MAIN@mander.xyz"

I have performed mastodon searches to fetch content as well, and the searches are now case-insensitive.

This commit would replace #2000. LemmyNet/lemmy-ui#525 can be reversed as users should be able to use uppercase names without problem. Issues #1955 and #1986 will be solved.

Comment on lines 146 to 148

sql_function!(fn lower(x: Text) -> Text);

Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be necessary, diesel already has a lower function, you just have to import it. https://docs.diesel.rs/diesel/query_dsl/trait.QueryDsl.html?search=lower#method.select

Copy link
Contributor Author

@Kradyz Kradyz Dec 19, 2021

Choose a reason for hiding this comment

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

I chose this method after reading a possibly oudated answer to diesel-rs/diesel#560, and this section of the manual.

I have tried to find specifically where I should load this function from, but in their internal tests and in the examples that I have found the function is defined locally wherever it is called.

The only place where I see the function defined under diesel:: is here, where it is defined to be used locally for a test. I don't know if it is correct to import the function from there.

I have checked the source of QueryDsl and I can't find the method defined there.

Copy link
Member

Choose a reason for hiding this comment

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

Ah my bad, I'd assumed it was one of their expression methods. That's right that you do have to add that sql_function then.

@dessalines dessalines merged commit 9f64872 into LemmyNet:main Dec 20, 2021
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.

3 participants