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

let jig handle utf8 issues a little more gracefully #352

Merged
merged 3 commits into from
Nov 3, 2022
Merged

let jig handle utf8 issues a little more gracefully #352

merged 3 commits into from
Nov 3, 2022

Conversation

n0nag0n
Copy link
Member

@n0nag0n n0nag0n commented Sep 16, 2022

No description provided.

@Rayne
Copy link
Member

Rayne commented Sep 17, 2022

Why did you enable pretty printing and unescaped slashes?

@ikkez
Copy link
Member

ikkez commented Sep 17, 2022

I've checked JSON_UNESCAPED_SLASHES and there's a potential risk not escaping forward slashed.. imagine someone just injects the jig JSON into a frontends <script> tag.. that opens a potential bug/security risk. Lets remove it..
I'm fine with the pretty print option.. that lets your easier inspect and debug the files manually... and since it was enabled before, lets keep it.

@n0nag0n
Copy link
Member Author

n0nag0n commented Sep 17, 2022

Pretty print was there before I touched it. As for the unescaped slashes, slashes don't have to be escaped in the JSON standard, PHP just does it. And with your scenario ikkez, the slashes are only for storing the json itself. When you decode the JSON, it would still have a </script> tag just like it was before. The slashes are purely a visual thing in the JSON spec. https://stackoverflow.com/questions/1580647/json-why-are-forward-slashes-escaped

@ikkez
Copy link
Member

ikkez commented Sep 18, 2022

I actually meant the case where a JSON is not decoded anymore by php.. imagine someone dropping the jig db json directly to javascript, because you can, as it is valid JSON:

$json = [[
  'value' => 'bar </script><script>alert(123)</script>',
]];
$f3->write('data.json', json_encode($json, JSON_UNESCAPED_SLASHES));

echo '<html><body><script>
  const data = '.$f3->read('data.json').';
</script></body></html>';

We could argue that nobody will probably do that, but i cannot be sure.. and as it is valid json, why not make it bullet proof if we can?!

@n0nag0n
Copy link
Member Author

n0nag0n commented Sep 19, 2022

Yeah, I suppose that edge case would apply, but then what you just outlined wouldn't be running through jig.php anyway. That'd be the users own fault they a directly reading/writing to a JSON file. If they did that, they hopefully would use templating at which point it would be auto escaped and if they didn't use templating, I pity their soul for not paying attention to all the security warnings that are out there about it..... 😆

@geniuswebtools
Copy link
Contributor

As a user of Fat-Free I would say that trying to allow us developers to access a JSON file created through the Jig library would be out of scope for the JSON files created through Jig. I realize that the contents represent a JSON object, however in development terms it's really of type Jig, and should be handled through the Jig class.

There are times where I will pull data out of MySQL and format the results so that I can use them as part of a JSON object, and I don't see any reason doing that with Jig would be a different process from the developer standpoint.

Thank you for your contributions to this project.

@n0nag0n
Copy link
Member Author

n0nag0n commented Sep 19, 2022

I was just going to add, I'm not married to the unescape slashes constant. If you want to remove it, my feelings really won't be hurt haha

@ikkez ikkez merged commit 6d92c4e into f3-factory:master Nov 3, 2022
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.

4 participants