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.

Sign up for GitHub

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

Jump to bottom

Another try at sanitize. #1208

Merged
merged 4 commits into from
Jun 12, 2023
Merged

Another try at sanitize. #1208

merged 4 commits into from
Jun 12, 2023

Conversation

dessalines
Copy link
Member

@dessalines dessalines commented Jun 12, 2023 •
edited
Loading

I'll be testing this shortly.

@dessalines dessalines changed the title Another try at this Another try at sanitize. Jun 12, 2023
@dessalines dessalines marked this pull request as ready for review June 12, 2023 17:00
@fheft
Copy link
Contributor

fheft commented Jun 12, 2023

To better verify this I was thinking about writing a few unit tests, if this is of any help. Though of course I won't commit them, as there's no test setup right now.

But I'm not really sure I fully understand what the sanitizeJson function should achieve. Obviously it should return a valid JSON string and it should not serialize prototypes. But why the serialize()? Will this not mess up everything, because it's blindly applied to the whole JSON string?

@dessalines
Copy link
Member Author

dessalines commented Jun 12, 2023 •
edited
Loading

The solution is from here

I apologize for not having a proper test setup, reddit caught us right in the middle of a large websocket -> http rework.

I have it deployed to https://voyager.lemmy.ml : v0.18.0-beta.4-3-g2653d65 , for testing / trying to break it.

Also agree its a horrendous hack, but this is the difficulty of me having to roll my own isomorphic support with inferno.

In the very long-term, @SleeplessOne1917 and I (and anyone else), want to rewrite lemmy-ui in rust, using leptos-rs, which has native isomorphic support.

@dessalines dessalines mentioned this pull request Jun 12, 2023
Closed
@dessalines dessalines marked this pull request as draft June 12, 2023 19:57
@dessalines
Copy link
Member Author

Adding another method, since we've verified that one also doesn't work.

@dessalines dessalines marked this pull request as ready for review June 12, 2023 20:04
@dessalines dessalines merged commit a605c72 into main Jun 12, 2023
dessalines added a commit that referenced this pull request Jun 12, 2023
@dessalines
Another try at sanitize. (#1208)
91f2b1c 
* Sanitize again.

* Adding sanitize json function.

* Using serialize instead.
makotech222 pushed a commit to hexbear-collective/lemmy-ui that referenced this pull request Jun 14, 2023
@dessalines @makotech222
Another try at sanitize. (LemmyNet#1208)
79c766d 
* Sanitize again.

* Adding sanitize json function.

* Using serialize instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SleeplessOne1917 SleeplessOne1917 SleeplessOne1917 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dessalines @fheft @SleeplessOne1917