You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "YegorKozlov (via GitHub)" <gi...@apache.org> on 2023/05/31 12:17:11 UTC
[GitHub] [sling-org-apache-sling-servlets-get] YegorKozlov opened a new pull request, #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
YegorKozlov opened a new pull request, #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11
The PR introduces and OSGi flag to enable exporting binary properties in base64
see https://issues.apache.org/jira/browse/SLING-11892
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] joerghoh commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1573328544
@YegorKozlov when you work with such a ``max_bytes_to_export`` setting, the operations in GET and POST are most likely not symmetric anymore; and if I understand you correctly, this is an important factor for you.
What about changing the approach and you provide a link to the binary as part of the JSON instead of dumping its content within the JSON?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] YegorKozlov commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "YegorKozlov (via GitHub)" <gi...@apache.org>.
YegorKozlov commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1579090911
@enapps-enorman I don't think the ".json.zip" idea is worth the effort. Sling GET servlet returns binary data by property path , e.g.` /nt_file/jcr:content/jcr:data ` and clients can fetch the json , the binaries and feed it to Sling POST Servlet.
My initial idea was to improve the _existing_ json export/import protocol and support base64. Inventing a new ".json.zip" exporter/importer is not worth the effort IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] joerghoh commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1572082183
I agree to @rombert here. But instead of a query parameter I would opt for a selector (e.g. "resource.withBinaries.json")
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] joerghoh commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1572338162
totally agree to @enapps-enorman, when binaries are involved, streaming is a must.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] enapps-enorman commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1583540309
Closing as requested
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] YegorKozlov commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "YegorKozlov (via GitHub)" <gi...@apache.org>.
YegorKozlov commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1572163648
@rombert @joerghoh I like the selector idea. It makes this feature dynamic per-request.
There is a related PR https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22 to make sense of the base64 data when importing content via Sling POST servlet, and the question is if it's okay to use a colon prefixed property to carry the data?
Sling POST Servlet ignores form properties starting with ":" with respect to content update.
What about importing content from json? If the GET servlet produces a JSON with colon-prefixed fields, should they be ignored when importing via Sling POST servlet? Or designating binary properties with a leading colon is just a naming convention?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] YegorKozlov commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "YegorKozlov (via GitHub)" <gi...@apache.org>.
YegorKozlov commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1577058092
> @YegorKozlov How would you feel about something like a ".json.zip" selector that exports a zip stream that contains the json content plus any binary properties as files in a zip file? That should get your the items and the binary contents in one round-trip without base64 encoding anything. Then the import could be enchanced to have specialized handling for that ".json.zip" extension to do the inverse.
@enapps-enorman That would simplify getting the data, but will not work for import. We can't do ".json.zip" because we need to "massage" the input json and remove protected properties (SLING-8375)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] rombert commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "rombert (via GitHub)" <gi...@apache.org>.
rombert commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1571909762
Note: CI failure is unrelated, filed https://issues.apache.org/jira/browse/SLING-11897
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] sonarcloud[bot] commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1576372573
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&metric=new_coverage&view=list) [61.5% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-servlets-get&pullRequest=11&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] enapps-enorman commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1572561639
> Base64 is enabled in jcr xml and so far it was okay.
If I am not mistaken, the XMLRenderer streams the xml text to the response. I don't believe it loads the whole xml document into memory before writing it to the response. Also, I think it reads the binary property value as an InputStream and then writes the base64 text directly to the output writer without loading the while file into memory. In that case, you only need enough memory for the small buffer that is consuming the InputStream while transforming it to base64.
That may not be possible with the JSON generator apis as it expects the whole string so it would likely need much more server side memory (something like ~140% the size of the original file) for the base64 conversion to a string.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] YegorKozlov commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "YegorKozlov (via GitHub)" <gi...@apache.org>.
YegorKozlov commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1572522399
Base64 is enabled in jcr xml and so far it was okay.
What if we have a configurable max_bytes_to_export osgi parameter ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] enapps-enorman commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1574012608
> So the idea was to fetch all the content in one go.
@YegorKozlov How would you feel about something like an ".infinity.json.zip" selector that exports a zip stream that contains the json content plus any binary properties as files in a zip file? That should get your the items and the binary contents in one round-trip without base64 encoding anything. Then the import could be enchanced to have specialized handling for that ".json.zip" extension to do the inverse.
For convenience, maybe even make the exported structure compatible with what is expected for ["Bundle Resources"](https://sling.apache.org/documentation/bundles/bundle-resources-extensions-bundleresource.html#defining-resources-through-json-1)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] enapps-enorman commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1579219412
@YegorKozlov Ok, in that case, do you have any objections to closing this PR without merging anything?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] YegorKozlov commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "YegorKozlov (via GitHub)" <gi...@apache.org>.
YegorKozlov commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1573452163
> What about changing the approach and you provide a link to the binary as part of the JSON instead of dumping its content within the JSON?
@joerghoh that's actually how I ended up doing it for now :)
My goal is to achieve symmetry between json export/import for the basic units of content in AEM like cq:Page, cq:Tag, etc. Since pages can contain images with inline data, those get lost and my idea was to transfer them as base64 in infinity.json.
I'm not aiming at a universal json round-tripping which is hardly possible. I just wanted to make it more symmetric.
So far I do N+1 GET requests to fetch the content, where N is the number of binary properties whose names start with a colon:
- one GET to fetch infinity.json with the resource content
- a GET to fetch each nt:file node with the binary data.
So the idea was to fetch all the content in one go.
@enapps-enorman I fully understand the memory consumption concern and it sounds serous enough to put this PR on hold. I'd like to see base64 in the json eventually, and I agree it should be streamed, not dumped.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] enapps-enorman commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1572335408
I'm still a bit concerned about this being a vector toward a denial-of-service attack.
For example, consider a scenario with a just a few large files in a folder. A swarm of "resource.withBinaries.json" requests to render that folder would load the base64 strings for all those files into memory which could exhaust all the server side memory.
Perhaps refactoring the JsonRenderer implementation to use javax.json.stream.JsonGenerator more directly to write the JSON data in a streaming way could mitigate that risk.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] enapps-enorman commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1577186781
> That would simplify getting the data, but will not work for import. We can't do ".json.zip" because we need to "massage" the input json and remove protected properties ([SLING-8375](https://issues.apache.org/jira/browse/SLING-8375))
@YegorKozlov I see, I guess I was under the impression you were downloading the content, manually changing it and then importing the changed content to the destination.
I thought you could do the same with .json.zip content by:
1. exporting the .json.zip content and save to a temp file
2. unzipping it
3. manually change whatever needs adjusted in the json content
4. re-zip the content
5. import the new .json.zip content to the destination
But perhaps, your client doesn't have the ability to zip/unzip (maybe javascript running in a web browser sandbox?)
Anyways, it was just a thought.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] YegorKozlov commented on pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "YegorKozlov (via GitHub)" <gi...@apache.org>.
YegorKozlov commented on PR #11:
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11#issuecomment-1579245397
@enapps-enorman the PR can be closed. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-servlets-get] enapps-enorman closed pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman closed pull request #11: SLING-11892: OSGi flag to enable exporting binary properties in JSON
URL: https://github.com/apache/sling-org-apache-sling-servlets-get/pull/11
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org