Use custom max depth for parsing of Blockly JSON by fisher-alice · Pull Request #66769 · code-dot-org/code-dot-org

fisher-alice · 2025-06-27T15:05:54Z

This is a follow-up to #66614 which added profanity/privacy filtering for open-ended project types geared for young users.

We started getting HoneyBadger reports of JSON::NestingError: nesting of 101 is too deep. These are caused by Sprite Lab programs whose JSON has nesting greater than 100 levels.

I checked a couple programs which triggered this error and the number of levels was between 100-300. Thus, I increased max_nesting level of JSON.parse from the default value of 100 to 999. I added a DCDO key in case we need to update this value quickly (thinking about when classes begin in the fall).

Links

HoneyBadger error report

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

Tests provide adequate coverage
Privacy and Security impacts have been assessed
Code is well-commented
New features are translatable or updates will not break translations
Relevant documentation has been added or updated
User impact is well-understood and desirable
Pull Request is labeled appropriately
Follow-up work items (including potential tech debt) are tracked and linked

molly-moen · 2025-06-27T16:17:00Z

      ShareFiltering.should_filter_program(with_indicator, 'playlab')
  end
+
+  def test_extract_text_blockly_handles_deeply_nested_json


I'm not sure these tests are necessary. They are basically just testing whether the max depth parameter works. Do they take a long time since they have to generate a long string?

I ran these locally and they didn't take long at all. I don't have a strong preference!

I don't think they are necessary!

Although if we add a state of "project too big" for a share failure they may make sense!

molly-moen · 2025-06-27T16:18:54Z


    # Texts will include field values, text values in block inputs, comments, and variable names.
-    json = JSON.parse(stripped)
+    json = JSON.parse(stripped, max_nesting: DCDO.get('share_filtering_blockly_json_max_depth', JSON_MAX_DEPTH))


If this fails, we will fail to load the project, right? This is maybe a longer-term fix but I wonder if we should still allow the project to load, but not allow it to be shared?

Or could we catch the error and just return that there was a share failure, something like "project too big"?

yeah! I was actually thinking about allowing the loading. But not sure about not allowing it to be shared...

I think as an intermediate step having it load but not be shareable (if that is easy to do here) seems like the right call. If we can't parse it, we don't know if it's shareable.

I think that makes sense. I'll add a new FailureType for PROGRAM_TOO_LARGE.
Just noting that in activites_controller, we have a rescue for WebPurify::TextTooLongError and the program is still allowed.

code-dot-org/dashboard/app/controllers/activities_controller.rb

Lines 61 to 71 in a98a055

if @level.game.sharing_filtered?

project_type = 'playlab'

begin

share_failure = ShareFiltering.find_share_failure(params[:program], locale, project_type)

rescue WebPurify::TextTooLongError, OpenURI::HTTPError, IO::EAGAINWaitReadable => exception

# If WebPurify or Geocoder fail, the program will be allowed, and we

# retain the share_filtering_error to log it alongside the level_source

# ID below.

share_filtering_error = exception

end

end

maybe worth checking with Sam?

Here's the commit that made programs too large not shareable: 2639841
However, I reverted since we checked with @samantha-code on Slack, and she said to let these programs still be shareable.

This reverts commit 2639841.

molly-moen · 2025-06-27T18:43:30Z

+    begin
+      json = JSON.parse(stripped, max_nesting: DCDO.get('share_filtering_blockly_json_max_depth', JSON_MAX_DEPTH))
+    rescue JSON::NestingError
+      CDO.log.warn "ShareFiltering.extract_text_blockly: JSON too deep after #{JSON_MAX_DEPTH} levels"


for my information: where does this go?

I'm not exactly sure! @bencodeorg I see you recently added a log in https://github.com/code-dot-org/code-dot-org/pull/65870/files. Do you know where we can access these afterwards?

I thought CloudWatch Logs? But that could be wrong.

molly-moen

Looks good!

fisher-alice added 2 commits June 27, 2025 15:04

use custom max depth for blockly json parsing

ae13f87

add unit tests

a98a055

fisher-alice marked this pull request as ready for review June 27, 2025 16:06

fisher-alice requested review from a team and molly-moen June 27, 2025 16:10

molly-moen reviewed Jun 27, 2025

View reviewed changes

fisher-alice added 5 commits June 27, 2025 18:28

remove unnec tests

10b5cee

add no sharing if program too large

2639841

Revert "add no sharing if program too large"

e72534c

This reverts commit 2639841.

if program too large, allow load/sharing of program

9791d3d

Merge branch 'staging' into alice/json-parsing

a000f67

fisher-alice requested a review from molly-moen June 27, 2025 18:41

molly-moen reviewed Jun 27, 2025

View reviewed changes

molly-moen approved these changes Jun 27, 2025

View reviewed changes

Merge branch 'staging' into alice/json-parsing

8efbbcf

fisher-alice merged commit 30fa6c8 into staging Jun 27, 2025
6 checks passed

fisher-alice deleted the alice/json-parsing branch June 27, 2025 22:39

	if @level.game.sharing_filtered?
	project_type = 'playlab'
	begin
	share_failure = ShareFiltering.find_share_failure(params[:program], locale, project_type)
	rescue WebPurify::TextTooLongError, OpenURI::HTTPError, IO::EAGAINWaitReadable => exception
	# If WebPurify or Geocoder fail, the program will be allowed, and we
	# retain the share_filtering_error to log it alongside the level_source
	# ID below.
	share_filtering_error = exception
	end
	end

Conversation

fisher-alice commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Links

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fisher-alice Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

molly-moen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fisher-alice commented Jun 27, 2025 •

edited

Loading

fisher-alice Jun 27, 2025 •

edited

Loading