Upgrade to mini_racer gem
#1791
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…un_script
WARNING: THIS IS A BREAKING CHANGE.
All parsers for manifest fields return a trivial data structure, made of
hash, array, string and number that are easy to serialize to another
language (in this case JS).
The XML parse, however, returns a Nokogiri::XML::Element object that
therubyracer exposed entirely to JavaScript (all methods were
accessible). Even worse, it would expose all objects returned by calls
to the object (e.g. Ruby arrays, Nokogiri::XML::NodeSet, ...).
This behavior had numerous issues:
1. it's exposing an unlimited Ruby interface to user controlled code
(device manifests), which is a security issue;
2. it's tying the Nokogiri interface to application uncontrolled code,
leading to the impossibility to ever use anything else, or having to
re-implement the whole interface (or at least what's used).
3. it's preventing us from moving away from the old abandonned
`therubyracer` gem to something maintained (i.e. `mini_racer`) that
don't expose non trivial objects.
After verifying on different production sites, we noticed that nobody
was using the XML parser, so I chose to change how we expose XML parsed
data to JavaScript.
We now serialize the Nokogiri::XML::Element as a trivial data structure
of the form:
```
var message = {
"name": "node_name",
"attributes": { "attr_name" => "attr_value" },
"children": [
// nested elements
]
}
```
In addition we still expose a `message.xpath(query)` function that
allows to query for nested data in a nicer way. It's only available on
the `message` variable and not available for nested children (the query
should directly return the node that you're interested in). The function
returns the same serialized data structure.
ysbaddaden
force-pushed
the
tech-debt/upgrade-to-mini-racer
branch
from
2e52836 to
4d36f44
Compare
ysbaddaden
commented
Dec 1, 2022
Comment on lines
+134
to
+136
| <Message> | ||
| <Patient name="Socrates" age="27"/> | ||
| </Message> |
There was a problem hiding this comment.
For example this XML document is serialized as:
{
"name": "Message",
"children": [
{
"name": "Patient",
"attributes": [{"name": "Socrates", "age": "17"}]
}
]
}
|
Let's wait until January to merge this, so that we send it to production when Julien is back from vacations. |
|
Looks good to me :) |
leandroradusky
approved these changes
Dec 13, 2022
leandroradusky
added a commit
that referenced
this pull request
Jan 17, 2023
* New feature: Upload measured concentrations for samples * Specs passing and small fixes * Enablin upload button only when files are present, remove file button, specs * Fixes PR review * All jquery removed, more PR review suggestions solved * Stashing changes * Stashing changes * Stashing changes * Stashing changes * Boxes reports entity working * Small fixes * Rework Box Label (#1819) * Remove Institution and Purpose labels to improve display * Truncate institution name * Add samples count * Box reports for LOD samples working * Small fix * Enabled PDF and SVG export in box reports * Visualization and print implmented for Challenge Boxes * Added computed AUC, TPR and FPR * automatic unblinding for samples with uploaded measurements (#1831) * Fixing of box label in UI (#1836) * Feature/1807 custom confirm modal (#1834) * Custom confirm modals for upload results form * Custom confirm modals for upload results form * Custom confirm modals for upload results form * Upgrade to `mini_racer` gem (#1791) * Migrate to mini_racer gem to replace the deprecated therubyracer gem * Fix: use MiniRacer::Context in ManifestFieldMapping * Fix: don't expose raw Ruby/Nokogiri objects in ManifestFieldMapping#run_script WARNING: THIS IS A BREAKING CHANGE. All parsers for manifest fields return a trivial data structure, made of hash, array, string and number that are easy to serialize to another language (in this case JS). The XML parse, however, returns a Nokogiri::XML::Element object that therubyracer exposed entirely to JavaScript (all methods were accessible). Even worse, it would expose all objects returned by calls to the object (e.g. Ruby arrays, Nokogiri::XML::NodeSet, ...). This behavior had numerous issues: 1. it's exposing an unlimited Ruby interface to user controlled code (device manifests), which is a security issue; 2. it's tying the Nokogiri interface to application uncontrolled code, leading to the impossibility to ever use anything else, or having to re-implement the whole interface (or at least what's used). 3. it's preventing us from moving away from the old abandonned `therubyracer` gem to something maintained (i.e. `mini_racer`) that don't expose non trivial objects. After verifying on different production sites, we noticed that nobody was using the XML parser, so I chose to change how we expose XML parsed data to JavaScript. We now serialize the Nokogiri::XML::Element as a trivial data structure of the form: ``` var message = { "name": "node_name", "attributes": { "attr_name" => "attr_value" }, "children": [ // nested elements ] } ``` In addition we still expose a `message.xpath(query)` function that allows to query for nested data in a nicer way. It's only available on the `message` variable and not available for nested children (the query should directly return the node that you're interested in). The function returns the same serialized data structure. * GHA: fix deprecated nodejs version warning * Styled slider, small fixes * Allow to download CSV template file and style fixes (#1835) * Allow to download CSV template file and style fixes * Fixes PR review * Basic functioning report for variants box * Merging completition * Merging completition * Merging completition * Merging competition * Fixes PR review Co-authored-by: Julien Portalier <julien@portalier.com>
leandroradusky
added a commit
that referenced
this pull request
Jan 19, 2023
* Feature/1809 samples reports entity (#1823) * New feature: Upload measured concentrations for samples * Specs passing and small fixes * Enablin upload button only when files are present, remove file button, specs * Fixes PR review * All jquery removed, more PR review suggestions solved * Stashing changes * Stashing changes * Stashing changes * Stashing changes * Boxes reports entity working * Small fixes * Fixes PR review * Fixes PR review * Fixes PR review * Fixes Julien's PR review * SamplesReportsController specs (#1839) * Boxes reports functioning (#1840) * New feature: Upload measured concentrations for samples * Specs passing and small fixes * Enablin upload button only when files are present, remove file button, specs * Fixes PR review * All jquery removed, more PR review suggestions solved * Stashing changes * Stashing changes * Stashing changes * Stashing changes * Boxes reports entity working * Small fixes * Rework Box Label (#1819) * Remove Institution and Purpose labels to improve display * Truncate institution name * Add samples count * Box reports for LOD samples working * Small fix * Enabled PDF and SVG export in box reports * Visualization and print implmented for Challenge Boxes * Added computed AUC, TPR and FPR * automatic unblinding for samples with uploaded measurements (#1831) * Fixing of box label in UI (#1836) * Feature/1807 custom confirm modal (#1834) * Custom confirm modals for upload results form * Custom confirm modals for upload results form * Custom confirm modals for upload results form * Upgrade to `mini_racer` gem (#1791) * Migrate to mini_racer gem to replace the deprecated therubyracer gem * Fix: use MiniRacer::Context in ManifestFieldMapping * Fix: don't expose raw Ruby/Nokogiri objects in ManifestFieldMapping#run_script WARNING: THIS IS A BREAKING CHANGE. All parsers for manifest fields return a trivial data structure, made of hash, array, string and number that are easy to serialize to another language (in this case JS). The XML parse, however, returns a Nokogiri::XML::Element object that therubyracer exposed entirely to JavaScript (all methods were accessible). Even worse, it would expose all objects returned by calls to the object (e.g. Ruby arrays, Nokogiri::XML::NodeSet, ...). This behavior had numerous issues: 1. it's exposing an unlimited Ruby interface to user controlled code (device manifests), which is a security issue; 2. it's tying the Nokogiri interface to application uncontrolled code, leading to the impossibility to ever use anything else, or having to re-implement the whole interface (or at least what's used). 3. it's preventing us from moving away from the old abandonned `therubyracer` gem to something maintained (i.e. `mini_racer`) that don't expose non trivial objects. After verifying on different production sites, we noticed that nobody was using the XML parser, so I chose to change how we expose XML parsed data to JavaScript. We now serialize the Nokogiri::XML::Element as a trivial data structure of the form: ``` var message = { "name": "node_name", "attributes": { "attr_name" => "attr_value" }, "children": [ // nested elements ] } ``` In addition we still expose a `message.xpath(query)` function that allows to query for nested data in a nicer way. It's only available on the `message` variable and not available for nested children (the query should directly return the node that you're interested in). The function returns the same serialized data structure. * GHA: fix deprecated nodejs version warning * Styled slider, small fixes * Allow to download CSV template file and style fixes (#1835) * Allow to download CSV template file and style fixes * Fixes PR review * Basic functioning report for variants box * Merging completition * Merging completition * Merging completition * Merging competition * Fixes PR review Co-authored-by: Julien Portalier <julien@portalier.com> * Merging reports into main & fixes * more bugfixes * Update app/helpers/samples_reports_helper.rb Co-authored-by: Julien Portalier <julien@portalier.com> * small fix Co-authored-by: Julien Portalier <julien@portalier.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #1790
I implemented a custom XML serializer to expose a trivial data structure to the JavaScript context because existing accessible solutions (e.g. Hash.from_xml()` from ActiveSupport) weren't very reliable (e.g. override attributes with contents), or required to introduce yet another dependency to handle something quite easy to implement in the end, and where we're in full control of the generated data structure (e.g. we can move to another XML library).