[jira] [Commented] (SLING-8869) SimpleHttpDistributionTransport does not refresh the secret for token based implementations.

[Ashish Chopra (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-8869?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16988538#comment-16988538
 ] 

Ashish Chopra commented on SLING-8869:
--

bq. If an Executor with a wrong pwd is cached then the only way to evict that 
executor is to recreate the DistributionTransportContext cache entirely. This 
happens currently because re-configuring a credential based secret provider 
will force the components referencing it to restart.
I share your understanding on this. Currently, new instances of 
{{RemoteDistributionPackageImporter}} will be created each time 
credential-providers is reconfigured (e.g., due to credential-update), ensuring 
that HTTP transport always works with most up-to-date credentials and executors 
with stale credentials aren't accessible (and are candidates for GC). This is 
true for both {{DistributionAgentFactory}} impls and 
{{RemoteDistributionPackageImporterFactory}}.
bq. [it works] only by luck.
Erm, to me it seems to be as-designed - unless there's another supported 
mechanism that can create/update credential-providers _without_ a corresponding 
creation of {{RemoteDistributionPackageImporter}} with updated credentials.
If such a mechanism exists, can you please explain it here for my better 
understanding?
bq. One way to handle this would be to evict the Executor based on the returned 
status code, 401 and 403.
Assuming my understanding is correct, this seems like overkill for a credential 
provider that contains static-secrets.
For dynamic-secrets, (even if there is 401/403 based executor-eviction, 
followed by token-reacquisition) since the secret-provider can't be explicitly 
asked for a token-update (which isn't possible because there's no such 
capability in the API [0]), we anyway need to rely on the secret-provider 
returning the most updated token when asked for.
In current proposal HTTP Transport impl doesn't concern itself with token 
expiry/regeneration/caching and dumbly asks for the credentials from 
secret-provider always, letting the secret-provider deal with all the 
complexities of returning most up-to-date token always, which it leverages for 
transport.

Can you please help me understand the benefit of a failing HTTP call as you 
propose?

[0] 
https://github.com/apache/sling-org-apache-sling-distribution-api/blob/master/src/main/java/org/apache/sling/distribution/transport/DistributionTransportSecretProvider.java#L45

> SimpleHttpDistributionTransport does not refresh the secret for token based 
> implementations.
> 
>
> Key: SLING-8869
> URL: https://issues.apache.org/jira/browse/SLING-8869
> Project: Sling
>  Issue Type: Bug
>  Components: Content Distribution
>Reporter: Mohit Arora
>Assignee: Timothee Maret
>Priority: Critical
> Fix For: Content Distribution Core 0.4.2
>
> Attachments: SLING-8869-new.patch, SLING-8869.patch
>
>  Time Spent: 20m
>  Remaining Estimate: 0h
>
> While saving the {{contextKeyExecutor}} in {{DistributionTransportContext}} 
> map, it is not expected that the secret associated with the executor could be 
> expired. This can happen in case of access token based implementations where 
> the token is expired after a certain period of time and has to be refreshed.
> The code to refresh the token is written in the secret provider but since the 
> executor is [cached in the 
> map|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/transport/impl/SimpleHttpDistributionTransport.java#L208]
>  the secrets are not refreshed. It works fine for credentials based secret 
> provider but not for access token based.
> cc - [~marett]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

encryption/decryption module

[Oliver Lietz]

Hi *,

I need some encryption/decryption functionalities in Sling similar to what is 
available in Karaf - but without dependencies to Karaf and Blueprint.
Do we want to host it here at Sling as Commons Crypto or should I make it an 
OPS4J Pax project?

Thanks,
O.

[jira] [Commented] (SLING-8869) SimpleHttpDistributionTransport does not refresh the secret for token based implementations.

[Mohit Arora (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-8869?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16988514#comment-16988514
 ] 

Mohit Arora commented on SLING-8869:


[~marett] I have raised PRs for both the approaches - 

https://github.com/apache/sling-org-apache-sling-distribution-core/pull/28
https://github.com/apache/sling-org-apache-sling-distribution-core/pull/29

cc - [~ashishc]

> SimpleHttpDistributionTransport does not refresh the secret for token based 
> implementations.
> 
>
> Key: SLING-8869
> URL: https://issues.apache.org/jira/browse/SLING-8869
> Project: Sling
>  Issue Type: Bug
>  Components: Content Distribution
>Reporter: Mohit Arora
>Assignee: Timothee Maret
>Priority: Critical
> Fix For: Content Distribution Core 0.4.2
>
> Attachments: SLING-8869-new.patch, SLING-8869.patch
>
>  Time Spent: 20m
>  Remaining Estimate: 0h
>
> While saving the {{contextKeyExecutor}} in {{DistributionTransportContext}} 
> map, it is not expected that the secret associated with the executor could be 
> expired. This can happen in case of access token based implementations where 
> the token is expired after a certain period of time and has to be refreshed.
> The code to refresh the token is written in the secret provider but since the 
> executor is [cached in the 
> map|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/transport/impl/SimpleHttpDistributionTransport.java#L208]
>  the secrets are not refreshed. It works fine for credentials based secret 
> provider but not for access token based.
> cc - [~marett]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[GitHub] [sling-org-apache-sling-distribution-core] mohiaror opened a new pull request #29: SLING-8869 SimpleHttpDistributionTransport does not refresh the secret for token based implementations.

[GitBox]

mohiaror opened a new pull request #29: SLING-8869 
SimpleHttpDistributionTransport does not refresh the secret for token based 
implementations.
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/29
 
 
   Do not cache the token based executor as we do not know when a token could 
be expired. Create a new executor with each request for token based 
implementation.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [sling-org-apache-sling-distribution-core] mohiaror opened a new pull request #28: SLING-8869 SimpleHttpDistributionTransport does not refresh the secret for token based implementations.

[GitBox]

mohiaror opened a new pull request #28: SLING-8869 
SimpleHttpDistributionTransport does not refresh the secret for token based 
implementations.
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/28
 
 
   Do not set the default authorization header at the time of creating the 
executor but delegate this responsibility to the issuer of the request such 
that everytime POST request is made, if the executor is token based, check for 
the expiry of the token and add the relevant header to the request.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[jira] [Commented] (SLING-8875) Create new Goal to create a FM Descriptor from a Maven Module

[Carsten Ziegeler (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-8875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16988511#comment-16988511
 ] 

Carsten Ziegeler commented on SLING-8875:
-

[~andysch] Good point with the naming, yes, we should rename it to 
IncludeArtifactMojo.java and also fix the comment at the top :)
If you need additional things that are specified in a feature model like repo 
init or configurations, the IncludeArtifact mojo allows to attach to an 
existing feature model.
So you can have a feature model in src/main/features with just repo init and 
configurations, and then add the main artifact to it. With your addition, you 
can then also add the compile time dependencies to it

> Create new Goal to create a FM Descriptor from a Maven Module
> -
>
> Key: SLING-8875
> URL: https://issues.apache.org/jira/browse/SLING-8875
> Project: Sling
>  Issue Type: New Feature
>  Components: Feature Model
>Affects Versions: slingfeature-maven-plugin 1.1.10
>Reporter: Andreas Schaefer
>Assignee: Andreas Schaefer
>Priority: Major
> Fix For: slingfeature-maven-plugin 1.1.12
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> In order to use FM Descriptor in projects modules should be creating and 
> exposing their FM Descriptors in local / remote Maven repositories. This way 
> an assembler can take these FMs and build up its Sling instance.
> To avoid making each module to write their own FM Descriptor we should have a 
> Goal in the SlingFeature Maven Plugin that does that automatically.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Commented] (SLING-5946) XSSAPI#encodeForJSString is not restrictive enough

[Ilguiz Latypov (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-5946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16988471#comment-16988471
 ] 

Ilguiz Latypov commented on SLING-5946:
---

[~radu], [~rombert], [~vladb]

> XSSAPI#encodeForJSString is not restrictive enough
> --
>
> Key: SLING-5946
> URL: https://issues.apache.org/jira/browse/SLING-5946
> Project: Sling
>  Issue Type: Bug
>  Components: Extensions
>Affects Versions: XSS Protection API 1.0.8
>Reporter: Vlad Bailescu
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: XSS Protection API 1.0.12
>
> Attachments: SLING_5946.patch
>
>
> Since SLING-5445, {{XSSAPI#encodeForJSString}} is no longer properly encoding 
> {{}} and 

[jira] [Commented] (SLING-8875) Create new Goal to create a FM Descriptor from a Maven Module

[Andreas Schaefer (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-8875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16988319#comment-16988319
 ] 

Andreas Schaefer commented on SLING-8875:
-

[~cziegeler] Sorry, I got confused between the "includeArtifact" property of 
the aggregate-features goal and the actual "include-artifact" goal. I mistook 
the class IncludeArtifact.java as the property class and overlooked that it is 
a Mojo (maybe we should rename it to "IncludeArtifactMojo.java" to avoid 
confusion).

I can merge the create-fm-descriptor into the include-artifact as they seem to 
do the same thing even though I think the naming might be misleading but that 
is fine if it is documented.
My code is based on the CP Converter code and I just took any code necessary to 
make it work. I will go ahead and merge my code into the include-artifact mojo 
and get ride of the unnecessary code.

In my code I was adding compile dependencies to the FM as they are not provided 
by other FMs (otherwise they would not be in compile scope). That said there is 
also the question if we want to allow the creator to add Repo-Init code 
snippets to this FM. I have a project where my code needs to have a System User 
created and set permissions on it.

> Create new Goal to create a FM Descriptor from a Maven Module
> -
>
> Key: SLING-8875
> URL: https://issues.apache.org/jira/browse/SLING-8875
> Project: Sling
>  Issue Type: New Feature
>  Components: Feature Model
>Affects Versions: slingfeature-maven-plugin 1.1.10
>Reporter: Andreas Schaefer
>Assignee: Andreas Schaefer
>Priority: Major
> Fix For: slingfeature-maven-plugin 1.1.12
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> In order to use FM Descriptor in projects modules should be creating and 
> exposing their FM Descriptors in local / remote Maven repositories. This way 
> an assembler can take these FMs and build up its Sling instance.
> To avoid making each module to write their own FM Descriptor we should have a 
> Goal in the SlingFeature Maven Plugin that does that automatically.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Comment Edited] (SLING-5946) XSSAPI#encodeForJSString is not restrictive enough

[Ilguiz Latypov (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-5946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16988288#comment-16988288
 ] 

Ilguiz Latypov edited comment on SLING-5946 at 12/4/19 11:32 PM:
-

Please reopen this due to an improper implementation risking exceptions in 
strict javascript engines.
|return source == null ? null : 
Encode.forJavaScript(source).replace("-", "u002D");|

 
 Substitutes on top of the encoder's result with the intent to correct the 
encoder are near-sighted (i.e. suffer from the context-free approach). If 
{{source}} had a backslash followed by a dash, i.e. {{raw }}, the 
{{forJavaScript}} call would properly change the backslash into 2 backslashes 
{{raw }} (this would result in the javascript engine turning 
the string literal back to {{raw }}). But the subsequent 
{{replace}} call will destroy the context of the second backslash, turning the 
string into {{raw u002D}} which would turn to {{raw u002D}} 
in the javascript engine's variable.

I argue for dropping the {{.replace()}} call (aiming at disabling malicious 
comment injection) here and encoding the opening angle bracket {{raw <}} as 
{{raw u003C}} in the {{Encode.forJavaScript}} implementation. This will 
protect against both the malicious comment injection and the injection of 
closing script tags {{raw script>}} forcing the javascript 
interpreter to drop out of the string literal context and drop out of the 
script context.

The existing prefixing of forward slashes with a backslash agrees with JSON but 
not with Javascript. It should be removed in favour of replacing just the 
opening angle bracket.
{noformat}
SingleEscapeCharacter :: one of
' " \ b f n r t v
{noformat}
[https://www.ecma-international.org/ecma-262/6.0/#sec-literals-string-literals]

[https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String#Escape_notation]


was (Author: ilatypov):
Please reopen this due to an improper implementation risking exceptions in 
strict javascript engines.
|return source == null ? null : 
Encode.forJavaScript(source).replace("-", "u002D");|

 
 Substitutes on top of the encoder's result with the intent to correct the 
encoder are near-sighted (i.e. suffer from the context-free approach). If 
{{source}} had a backslash followed by a dash, i.e. {{raw -}}, the 
{{forJavaScript}} call would properly change the backslash into 2 backslashes 
{{raw -}} (this would result in the javascript engine turning the 
string literal back to {{raw -}}). But the subsequent {{replace}} call 
will destroy the context of the second backslash, turning the string into {{raw 
u002D}} which would turn to {{raw u002D}} in the javascript 
engine's variable.

I argue for dropping the {{.replace()}} call here and encoding the opening 
angle bracket {{raw <}} as {{raw u003C}} in the {{Encode.forJavaScript}} 
implementation. This will also protect against the closing script tags {{raw 
script>}} forcing the javascript interpreter to drop out of the 
string literal context and drop out of the script context.

The existing prefixing of forward slashes with a backslash agrees with JSON but 
not with Javascript. It should be removed in favour of replacing just the 
opening angle bracket.
{noformat}
SingleEscapeCharacter :: one of
' " \ b f n r t v
{noformat}
[https://www.ecma-international.org/ecma-262/6.0/#sec-literals-string-literals]

[https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String#Escape_notation]

> XSSAPI#encodeForJSString is not restrictive enough
> --
>
> Key: SLING-5946
> URL: https://issues.apache.org/jira/browse/SLING-5946
> Project: Sling
>  Issue Type: Bug
>  Components: Extensions
>Affects Versions: XSS Protection API 1.0.8
>Reporter: Vlad Bailescu
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: XSS Protection API 1.0.12
>
> Attachments: SLING_5946.patch
>
>
> Since SLING-5445, {{XSSAPI#encodeForJSString}} is no longer properly encoding 
> {{}} and 

[jira] [Comment Edited] (SLING-5946) XSSAPI#encodeForJSString is not restrictive enough

[Ilguiz Latypov (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-5946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16988288#comment-16988288
 ] 

Ilguiz Latypov edited comment on SLING-5946 at 12/4/19 11:30 PM:
-

Please reopen this due to an improper implementation risking exceptions in 
strict javascript engines.
|return source == null ? null : 
Encode.forJavaScript(source).replace("-", "u002D");|

 
 Substitutes on top of the encoder's result with the intent to correct the 
encoder are near-sighted (i.e. suffer from the context-free approach). If 
{{source}} had a backslash followed by a dash, i.e. {{raw -}}, the 
{{forJavaScript}} call would properly change the backslash into 2 backslashes 
{{raw -}} (this would result in the javascript engine turning the 
string literal back to {{raw -}}). But the subsequent {{replace}} call 
will destroy the context of the second backslash, turning the string into {{raw 
u002D}} which would turn to {{raw u002D}} in the javascript 
engine's variable.

I argue for dropping the {{.replace()}} call here and encoding the opening 
angle bracket {{raw <}} as {{raw u003C}} in the {{Encode.forJavaScript}} 
implementation. This will also protect against the closing script tags {{raw 
script>}} forcing the javascript interpreter to drop out of the 
string literal context and drop out of the script context.

The existing prefixing of forward slashes with a backslash agrees with JSON but 
not with Javascript. It should be removed in favour of replacing just the 
opening angle bracket.
{noformat}
SingleEscapeCharacter :: one of
' " \ b f n r t v
{noformat}
[https://www.ecma-international.org/ecma-262/6.0/#sec-literals-string-literals]

[https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String#Escape_notation]


was (Author: ilatypov):
Please reopen this due to an improper implementation risking exceptions in 
strict javascript engines.
|return source == null ? null : Encode.forJavaScript(source).replace("\\-", 
"\\u002D");|

 
 Substitutes on top of the encoder's result with the intent to correct the 
encoder are near-sighted (i.e. suffer from the context-free approach). If 
{{source}} had a backslash followed by a dash, i.e. {{raw \--}}, the 
{{forJavaScript}} call would properly change the backslash into 2 backslashes 
{{raw \\}} (this would result in the javascript engine turning the string 
literal back to {{raw \-}}). But the subsequent {{replace}} call will destroy 
the context of the second backslash, turning the string into {{raw \\u002D}} 
which would turn to {{raw \u002D}} in the javascript engine's variable.

I argue for dropping the {{.replace()}} call here and encoding the opening 
angle bracket {{raw <}} as {{raw \u003C}} in the {{Encode.forJavaScript}} 
implementation. This will also protect against the closing script tags {{raw 
}} forcing the javascript interpreter to drop out of the string 
literal context and drop out of the script context.

The existing prefixing of forward slashes with a backslash agrees with JSON but 
not with Javascript. It should be removed in favour of replacing just the 
opening angle bracket.
{noformat}
SingleEscapeCharacter :: one of
' " \ b f n r t v
{noformat}
[https://www.ecma-international.org/ecma-262/6.0/#sec-literals-string-literals]

[https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String#Escape_notation]

> XSSAPI#encodeForJSString is not restrictive enough
> --
>
> Key: SLING-5946
> URL: https://issues.apache.org/jira/browse/SLING-5946
> Project: Sling
>  Issue Type: Bug
>  Components: Extensions
>Affects Versions: XSS Protection API 1.0.8
>Reporter: Vlad Bailescu
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: XSS Protection API 1.0.12
>
> Attachments: SLING_5946.patch
>
>
> Since SLING-5445, {{XSSAPI#encodeForJSString}} is no longer properly encoding 
> {{}} and 

[jira] [Comment Edited] (SLING-5946) XSSAPI#encodeForJSString is not restrictive enough

[Ilguiz Latypov (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-5946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16988288#comment-16988288
 ] 

Ilguiz Latypov edited comment on SLING-5946 at 12/4/19 11:27 PM:
-

Please reopen this due to an improper implementation risking exceptions in 
strict javascript engines.
|return source == null ? null : Encode.forJavaScript(source).replace("\\-", 
"\\u002D");|

 
 Substitutes on top of the encoder's result with the intent to correct the 
encoder are near-sighted (i.e. suffer from the context-free approach). If 
{{source}} had a backslash followed by a dash, i.e. {{raw \--}}, the 
{{forJavaScript}} call would properly change the backslash into 2 backslashes 
{{raw \\}} (this would result in the javascript engine turning the string 
literal back to {{raw \-}}). But the subsequent {{replace}} call will destroy 
the context of the second backslash, turning the string into {{raw \\u002D}} 
which would turn to {{raw \u002D}} in the javascript engine's variable.

I argue for dropping the {{.replace()}} call here and encoding the opening 
angle bracket {{raw <}} as {{raw \u003C}} in the {{Encode.forJavaScript}} 
implementation. This will also protect against the closing script tags {{raw 
}} forcing the javascript interpreter to drop out of the string 
literal context and drop out of the script context.

The existing prefixing of forward slashes with a backslash agrees with JSON but 
not with Javascript. It should be removed in favour of replacing just the 
opening angle bracket.
{noformat}
SingleEscapeCharacter :: one of
' " \ b f n r t v
{noformat}
[https://www.ecma-international.org/ecma-262/6.0/#sec-literals-string-literals]

[https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String#Escape_notation]


was (Author: ilatypov):
Please reopen this due to an improper implementation risking exceptions in 
strict javascript engines.

|return source == null ? null : 
Encode.forJavaScript(source).replace("-", "u002D");|
 
Substitutes on top of the encoder's result with the intent to correct the 
encoder are near-sighted (i.e. suffer from the context-free approach). If 
{{source}} had a backslash followed by a dash {{raw -}}, the 
{{forJavaScript}} call would properly change the backslash into 2 backslashes 
{{raw -}} (this would result in the javascript engine turning the 
string literal back to {{raw -}}). But the subsequent {{replace}} call 
will destroy the context of the second backslash, turning the string into {{raw 
u002D}} which would turn to {{raw u002D}} in the javascript 
engine's variable.

I argue for dropping the {{.replace()}} call here and encoding the opening 
angle bracket {{raw <}} as {{raw u003C}} in the {{Encode.forJavaScript}} 
implementation. This will also protect against the closing script tags {{raw 
/script>}} forcing the javascript interpreter to drop out of the string 
literal context and drop out of the script context.

The existing prefixing of forward slashes with a backslash agrees with JSON but 
not with Javascript. It should be removed in favour of replacing just the 
opening angle bracket.

{noformat}
SingleEscapeCharacter :: one of
' " \ b f n r t v
{noformat}
https://www.ecma-international.org/ecma-262/6.0/#sec-literals-string-literals

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String#Escape_notation


> XSSAPI#encodeForJSString is not restrictive enough
> --
>
> Key: SLING-5946
> URL: https://issues.apache.org/jira/browse/SLING-5946
> Project: Sling
>  Issue Type: Bug
>  Components: Extensions
>Affects Versions: XSS Protection API 1.0.8
>Reporter: Vlad Bailescu
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: XSS Protection API 1.0.12
>
> Attachments: SLING_5946.patch
>
>
> Since SLING-5445, {{XSSAPI#encodeForJSString}} is no longer properly encoding 
> {{}} and 

[jira] [Comment Edited] (SLING-5946) XSSAPI#encodeForJSString is not restrictive enough

[Ilguiz Latypov (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-5946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16988288#comment-16988288
 ] 

Ilguiz Latypov edited comment on SLING-5946 at 12/4/19 11:26 PM:
-

Please reopen this due to an improper implementation risking exceptions in 
strict javascript engines.

|return source == null ? null : 
Encode.forJavaScript(source).replace("-", "u002D");|
 
Substitutes on top of the encoder's result with the intent to correct the 
encoder are near-sighted (i.e. suffer from the context-free approach). If 
{{source}} had a backslash followed by a dash {{raw -}}, the 
{{forJavaScript}} call would properly change the backslash into 2 backslashes 
{{raw -}} (this would result in the javascript engine turning the 
string literal back to {{raw -}}). But the subsequent {{replace}} call 
will destroy the context of the second backslash, turning the string into {{raw 
u002D}} which would turn to {{raw u002D}} in the javascript 
engine's variable.

I argue for dropping the {{.replace()}} call here and encoding the opening 
angle bracket {{raw <}} as {{raw u003C}} in the {{Encode.forJavaScript}} 
implementation. This will also protect against the closing script tags {{raw 
/script>}} forcing the javascript interpreter to drop out of the string 
literal context and drop out of the script context.

The existing prefixing of forward slashes with a backslash agrees with JSON but 
not with Javascript. It should be removed in favour of replacing just the 
opening angle bracket.

{noformat}
SingleEscapeCharacter :: one of
' " \ b f n r t v
{noformat}
https://www.ecma-international.org/ecma-262/6.0/#sec-literals-string-literals

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String#Escape_notation



was (Author: ilatypov):
Please reopen this due to an improper implementation risking exceptions in 
strict javascript engines.
| return source == null ? null : Encode.forJavaScript(source).replace("\\-", 
"\\u002D");|
 
Substitutes on top of the encoder's result with the intent to correct the 
encoder are near-sighted (i.e. suffer from the context-free approach). If 
{{source}} had a backslash followed by a dash {{raw \-}}, the {{forJavaScript}} 
call would properly change the backslash into 2 backslashes {{raw \\-}} (this 
would result in the javascript engine turning the string literal back to {{raw 
\-}}). But the subsequent {{replace}} call will destroy the context of the 
second backslash, turning the string into {{raw \\u002D}} which would turn to 
{{raw \u002D}} in the javascript engine's variable.

I argue for dropping the {{.replace()}} call here and encoding the opening 
angle bracket {{raw <}} as {{raw \u003C}} in the Encode.forJavaScript() 
implementation. This will also protect against the closing script tags {{raw 
}} forcing the javascript interpreter to drop out of the string 
literal context and drop out of the script context. The existing prefixing of 
forward slashes with a backslash agrees with JSON but not with Javascript. It 
should be removed in favour of replacing just the opening angle bracket.

> XSSAPI#encodeForJSString is not restrictive enough
> --
>
> Key: SLING-5946
> URL: https://issues.apache.org/jira/browse/SLING-5946
> Project: Sling
>  Issue Type: Bug
>  Components: Extensions
>Affects Versions: XSS Protection API 1.0.8
>Reporter: Vlad Bailescu
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: XSS Protection API 1.0.12
>
> Attachments: SLING_5946.patch
>
>
> Since SLING-5445, {{XSSAPI#encodeForJSString}} is no longer properly encoding 
> {{}} and 

[jira] [Commented] (SLING-5946) XSSAPI#encodeForJSString is not restrictive enough

[Ilguiz Latypov (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-5946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16988288#comment-16988288
 ] 

Ilguiz Latypov commented on SLING-5946:
---

Please reopen this due to an improper implementation risking exceptions in 
strict javascript engines.
| return source == null ? null : Encode.forJavaScript(source).replace("\\-", 
"\\u002D");|
 
Substitutes on top of the encoder's result with the intent to correct the 
encoder are near-sighted (i.e. suffer from the context-free approach). If 
{{source}} had a backslash followed by a dash {{raw \-}}, the {{forJavaScript}} 
call would properly change the backslash into 2 backslashes {{raw \\-}} (this 
would result in the javascript engine turning the string literal back to {{raw 
\-}}). But the subsequent {{replace}} call will destroy the context of the 
second backslash, turning the string into {{raw \\u002D}} which would turn to 
{{raw \u002D}} in the javascript engine's variable.

I argue for dropping the {{.replace()}} call here and encoding the opening 
angle bracket {{raw <}} as {{raw \u003C}} in the Encode.forJavaScript() 
implementation. This will also protect against the closing script tags {{raw 
}} forcing the javascript interpreter to drop out of the string 
literal context and drop out of the script context. The existing prefixing of 
forward slashes with a backslash agrees with JSON but not with Javascript. It 
should be removed in favour of replacing just the opening angle bracket.

> XSSAPI#encodeForJSString is not restrictive enough
> --
>
> Key: SLING-5946
> URL: https://issues.apache.org/jira/browse/SLING-5946
> Project: Sling
>  Issue Type: Bug
>  Components: Extensions
>Affects Versions: XSS Protection API 1.0.8
>Reporter: Vlad Bailescu
>Assignee: Robert Munteanu
>Priority: Major
> Fix For: XSS Protection API 1.0.12
>
> Attachments: SLING_5946.patch
>
>
> Since SLING-5445, {{XSSAPI#encodeForJSString}} is no longer properly encoding 
> {{}} and 

[jira] [Commented] (SLING-8869) SimpleHttpDistributionTransport does not refresh the secret for token based implementations.

[Timothee Maret (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-8869?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16987967#comment-16987967
 ] 

Timothee Maret commented on SLING-8869:
---

{quote}It works fine for credentials based secret provider but not for access 
token based.
{quote}
True, but only by luck. 

If an Executor with a wrong pwd is cached then the only way to evict that 
executor is to recreate the DistributionTransportContext cache entirely. This 
happens currently because re-configuring a credential based secret provider 
will force the components referencing it to restart.

One way to handle this would be to evict the Executor based on the returned 
status code, 401 and 403.

 

 

Please open PRs instead of attaching patches, it's much easier to 
review/comment :)

> SimpleHttpDistributionTransport does not refresh the secret for token based 
> implementations.
> 
>
> Key: SLING-8869
> URL: https://issues.apache.org/jira/browse/SLING-8869
> Project: Sling
>  Issue Type: Bug
>  Components: Content Distribution
>Reporter: Mohit Arora
>Assignee: Timothee Maret
>Priority: Critical
> Fix For: Content Distribution Core 0.4.2
>
> Attachments: SLING-8869-new.patch, SLING-8869.patch
>
>
> While saving the {{contextKeyExecutor}} in {{DistributionTransportContext}} 
> map, it is not expected that the secret associated with the executor could be 
> expired. This can happen in case of access token based implementations where 
> the token is expired after a certain period of time and has to be refreshed.
> The code to refresh the token is written in the secret provider but since the 
> executor is [cached in the 
> map|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/transport/impl/SimpleHttpDistributionTransport.java#L208]
>  the secrets are not refreshed. It works fine for credentials based secret 
> provider but not for access token based.
> cc - [~marett]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

RE: [VOTE] Release Apache Sling Content Distribution Journal Core 0.1.6

[Stefan Seifert]

+1

Re: sling.servlet.resolution = strict ?

[Bertrand Delacretaz]

Hi,

On Wed, Dec 4, 2019 at 2:48 PM Carsten Ziegeler  wrote:
> Am 04.12.2019 um 14:42 schrieb Bertrand Delacretaz:
> >...org.apache.sling.servlets.resolver;strict.mode.support:Version=1.0.0
> >
>
>... 
>osgi.extender;osgi.extender="org.apache.sling.servlets.resolver";version:Version="1.1"

Your variant is simpler indeed, just increment the servlets.resolver
version capability.

> ...I think, what we can do is provide a new version of the sling servlet
> annotations which provide the new mode property. And if these
> annotations are used, we require version 1.1 of the capability...

Makes sense - it won't cover cases where people set the service
properties without using the annotations, but in such a case it's fair
to expect them to also set the Require-Capability accordingly.

Thanks for your input, I think I'm all set for now!

-Bertrand

Re: sling.servlet.resolution = strict ?

[Carsten Ziegeler]

Am 04.12.2019 um 14:42 schrieb Bertrand Delacretaz:


We can discuss separately whether "strict" is the right base name.
Naming is hard ;-)



:)


1) When this is implemented, the o.a.s.servlets.resolver bundle gets
an additional (static, in pom.xml) value for its Provide-Capability
header. Suggested value:

   org.apache.sling.servlets.resolver;strict.mode.support:Version=1.0.0



That's one option. The other option is to increase the version for the 
already existing capability.

Right now the bundle provides:

osgi.extender;osgi.extender="org.apache.sling.servlets.resolver";version:Version="1.0"

As we're adding new functionality, we could update that to

osgi.extender;osgi.extender="org.apache.sling.servlets.resolver";version:Version="1.1"

Existing bundles continue to work (they should require version 1.0), 
new/updated bundles making use of the new functionality, should require 
version 1.1.

I'm saying "should" here as we can't enfore it in all cases.


2) The bnd functionality is modified to add the corresponding
Require-Capability header (require strict.mode.support >= 1.0.0) when
building bundles that contain at least one servlet with both a
sling.servlet.paths and a sling.servlet.resolution service property.
At this stage I don't know where this needs to be implemented.


I'm not sure if that is possible. If so, that would be nice.

I think, what we can do is provide a new version of the sling servlet 
annotations which provide the new mode property. And if these 
annotations are used, we require version 1.1 of the capability.
Now, that means - regardless of the value used for the mode (strict or 
default) - the newer resolver bundle is required; but that's probably 
fine as the only reason to update to the new annotations is to use the 
new functionality.


Regards
Carsten
--
Carsten Ziegeler
Adobe Research Switzerland
cziege...@apache.org

Re: sling.servlet.resolution = strict ?

[Konrad Windszus]

For b) we can use 
https://osgi.org/javadoc/osgi.annotation/7.0.0/org/osgi/annotation/bundle/Requirement.html
 

 as meta annotation on a new servlet annotation.
That way, bnd will automatically add the according headers to the bundle once 
such a servlet annotation is found.

Konrad

> On 4. Dec 2019, at 14:42, Bertrand Delacretaz  wrote:
> 
> Hi,
> 
> On Wed, Dec 4, 2019 at 11:00 AM Carsten Ziegeler  wrote:
>> ...The servlets resolver bundle
>> already provides such a capability, so we can build on that one.
>> If developers use the annotations, we can use the bnd functionality to
>> automatically generate the requirement if the annotation using the new
>> functionality is used
> 
> Thank you Carsten and Konrad for your comments (and Radu for the +1s),
> I agree with the need to be fully backwards compatible and avoid
> surprises. Using OSGi capabilities makes total sense.
> 
> I haven't worked much with OSGi capabilities, can you confirm my
> understanding of how they should be used and generated?
> 
> We can discuss separately whether "strict" is the right base name.
> Naming is hard ;-)
> 
> 1) When this is implemented, the o.a.s.servlets.resolver bundle gets
> an additional (static, in pom.xml) value for its Provide-Capability
> header. Suggested value:
> 
>  org.apache.sling.servlets.resolver;strict.mode.support:Version=1.0.0
> 
> 2) The bnd functionality is modified to add the corresponding
> Require-Capability header (require strict.mode.support >= 1.0.0) when
> building bundles that contain at least one servlet with both a
> sling.servlet.paths and a sling.servlet.resolution service property.
> At this stage I don't know where this needs to be implemented.
> 
> -Bertrand
> 
> (note to self; 
> https://osgi.org/specification/osgi.core/7.0.0/framework.module.html#d0e2821
> is the Capabilities spec)

Re: sling.servlet.resolution = strict ?

[Bertrand Delacretaz]

Hi,

On Wed, Dec 4, 2019 at 11:00 AM Carsten Ziegeler  wrote:
> ...The servlets resolver bundle
> already provides such a capability, so we can build on that one.
> If developers use the annotations, we can use the bnd functionality to
> automatically generate the requirement if the annotation using the new
> functionality is used

Thank you Carsten and Konrad for your comments (and Radu for the +1s),
I agree with the need to be fully backwards compatible and avoid
surprises. Using OSGi capabilities makes total sense.

I haven't worked much with OSGi capabilities, can you confirm my
understanding of how they should be used and generated?

We can discuss separately whether "strict" is the right base name.
Naming is hard ;-)

1) When this is implemented, the o.a.s.servlets.resolver bundle gets
an additional (static, in pom.xml) value for its Provide-Capability
header. Suggested value:

  org.apache.sling.servlets.resolver;strict.mode.support:Version=1.0.0

2) The bnd functionality is modified to add the corresponding
Require-Capability header (require strict.mode.support >= 1.0.0) when
building bundles that contain at least one servlet with both a
sling.servlet.paths and a sling.servlet.resolution service property.
At this stage I don't know where this needs to be implemented.

-Bertrand

(note to self; 
https://osgi.org/specification/osgi.core/7.0.0/framework.module.html#d0e2821
is the Capabilities spec)

[jira] [Commented] (SLING-8869) SimpleHttpDistributionTransport does not refresh the secret for token based implementations.

[Ashish Chopra (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-8869?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16987757#comment-16987757
 ] 

Ashish Chopra commented on SLING-8869:
--

[~mohiaror], may I suggest raising a PR for SCD maintainers (and broader Sling 
community) to review and merge? Thanks!

\cc: [~marett]

> SimpleHttpDistributionTransport does not refresh the secret for token based 
> implementations.
> 
>
> Key: SLING-8869
> URL: https://issues.apache.org/jira/browse/SLING-8869
> Project: Sling
>  Issue Type: Bug
>  Components: Content Distribution
>Reporter: Mohit Arora
>Assignee: Timothee Maret
>Priority: Critical
> Fix For: Content Distribution Core 0.4.2
>
> Attachments: SLING-8869-new.patch, SLING-8869.patch
>
>
> While saving the {{contextKeyExecutor}} in {{DistributionTransportContext}} 
> map, it is not expected that the secret associated with the executor could be 
> expired. This can happen in case of access token based implementations where 
> the token is expired after a certain period of time and has to be refreshed.
> The code to refresh the token is written in the secret provider but since the 
> executor is [cached in the 
> map|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/transport/impl/SimpleHttpDistributionTransport.java#L208]
>  the secrets are not refreshed. It works fine for credentials based secret 
> provider but not for access token based.
> cc - [~marett]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Commented] (SLING-8868) DistributionPackage built using DistributionPackageBuilder.getPackage doesn't contain correct DIstributionPackageInfo

[Ashish Chopra (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-8868?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16987754#comment-16987754
 ] 

Ashish Chopra commented on SLING-8868:
--

[~marett], I've updated the PR [0] to address sonarqube and coverage concerns 
reported in the previous iteration.

Please review and provide feedback at [0] and merge in case you feel this is 
fine. Thanks!

[0] https://github.com/apache/sling-org-apache-sling-distribution-core/pull/27

> DistributionPackage built using DistributionPackageBuilder.getPackage doesn't 
> contain correct DIstributionPackageInfo
> -
>
> Key: SLING-8868
> URL: https://issues.apache.org/jira/browse/SLING-8868
> Project: Sling
>  Issue Type: Bug
>  Components: Content Distribution
>Affects Versions: Content Distribution Core 0.4.0
>Reporter: Ashish Chopra
>Priority: Major
> Fix For: Content Distribution Core 0.4.2
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> {{DIstributionPackage}} as returned by 
> {{DistributionPackageBuilder.createPackage}} contains metadata of the package 
> within it, which can be retrieved via {{DIstributionPackage.getInfo()}}. 
> {{DistributionPackage.getId}} returns the unique identifier for the created 
> package, which can later be retrieved via the ID and 
> {{DistributionPackageBuilder.getPackage}} API.
> However, when the {{DistributionPackage}} is retrieved, the 
> {{DistributionPackageInfo}} returned by {{.getInfo}} method doesn't contain 
> correct metadata of the package.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Commented] (SLING-8854) In-file and In-memory queue-providers for Forward Distribution report incorrect value for processing "Attempts"

[Ashish Chopra (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-8854?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16987745#comment-16987745
 ] 

Ashish Chopra commented on SLING-8854:
--

[~marett], comments on SLING-8743 indicate that separating the implementation 
of {{DistributionQueueProvider}} and {{DistributionQueue}} with that of a 
{{DistributionQueueProcessor}} is the correct semantic.
(the implementation of the queue is expected to reside in one bundle - such as 
{{sling.distribution.core}}, whereas its processing is expected to exist in 
another bundle).

Given above, we shouldn't think of the queue-processor to be coupled with queue 
and queue-providers and thus can't expect the queue-processors to work in 
cohorts with queue and queue-providers to maintain the processing attempts.

IMHO the resolution of this issue would be to:
* either change the behaviour of {{DistributionQueue.getHead}} such that it 
doesn't increment the processing count automatically and add a new method to 
the SPI which {{DistributionQueueProcessor}} can invoke to let the queue 
capture a processing attempt ([approach #1 
here|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16982396=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982396])
** fwiw, this also aligns with the opinion you expressed that 
[{{DistributionQueue.getHead}} has no business incrementing processing count 
and should be side-effect 
free|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16982155=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982155].
* or add a new {{DistributionQueue.peek}} for introspection, expected to be 
used by components interested in inspecting and reporting queue-statuses 
([approach #2 
here|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16982396=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982396])
** this keeps the behaviour of {{DistributionQueue.getHead}} intact - if 
required for backward-compatibility reasons - but IMHO is slightly more 
confusing than the first option.

IMVHO approach which expects queue-provider, queue and queue-processors is too 
much coupling and severely reduces the fluency of the API.

Looking forward to your feedback on the alternative PRs [I shared 
here|https://issues.apache.org/jira/browse/SLING-8854?focusedCommentId=16982396=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982396].
 Thanks!

> In-file and In-memory queue-providers for Forward Distribution report 
> incorrect value for processing "Attempts"
> ---
>
> Key: SLING-8854
> URL: https://issues.apache.org/jira/browse/SLING-8854
> Project: Sling
>  Issue Type: Bug
>  Components: Content Distribution
>Affects Versions: Content Distribution Core 0.4.0
>Reporter: Ashish Chopra
>Assignee: Timothee Maret
>Priority: Major
> Attachments: 
> 0001-SLING-8854-In-file-and-In-memory-queue-providers-for.patch
>
>  Time Spent: 20m
>  Remaining Estimate: 0h
>
> {{ForwardDistributionAgentFactory}} allows specifying "In-Memory" and 
> "In-File" queue-providers [0]. Both these implementations are provided by 
> {{SimpleDistributionQueue}} [1].
> While responding to a queue-processing request, the head of the queue is 
> fetched to create a {{DistributionQueueEntry}}, which encapsulates a 
> {{DistributionQueueItemStatus}}. While retrieving the queue-head-entry, the 
> number-of-attempts are also incremented by 1, implying _one more_ 
> processing-attempt to have happened on a given queue-item. [2]
> The trouble arises from the fact that {{SimpleDistributionQueue#getHead}} 
> ends up being called from at least three other places:
> # Queue status Health Check [3], which is a periodic job
> # UI (via {{ExtendedDistributionServiceResourceProvider}}) [4]
> # Distribution Queue status MBean [5]
> Out of these, #1 is most troublesome - because of its periodic nature, the 
> number of Attempts for a queue-item that's being processed (or, is blocked) 
> will keep on increasing without corresponding dequeue-attempts.
> Due to #2, everytime the queue status is delivered to the UI, the attempt 
> would again increase. Similar would be the case of MBean invocations.
> This needs to be addressed. Notably, "Sling Jobs" based queue doesn't suffer 
> from this issue because it relies on retry-counts as maintained by Sling Job 
> Execution Engine.
> [0] 
> https://github.com/apache/sling-org-apache-sling-distribution-core/blob/deb3d2ae33c4f4678c8503091a9fffdbb141e569/src/main/java/org/apache/sling/distribution/agent/impl/ForwardDistributionAgentFactory.java#L258-L262
> [1] 
> 

[jira] [Commented] (SLING-8853) Develop an Active variant of ResourceQueue and make it available as possible queue.provider for ForwardDistributionAgentFactory

[Ashish Chopra (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-8853?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16987743#comment-16987743
 ] 

Ashish Chopra commented on SLING-8853:
--

I've updated the PR for this issue [0] with the tests for ActiveResourceQueue 
and its corresponding provider's updates.

While I [couldn't get the existing Pax Exam based tests to 
run|https://issues.apache.org/jira/browse/SLING-8853?focusedCommentId=16982140=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982140]
 still, I chose to take a different approach and write the test using Mock 
Sling and Mock Sling Oak test-tooling.

[~marett], would request you to provide feedback on [0] and merge.

[0] https://github.com/apache/sling-org-apache-sling-distribution-core/pull/24

> Develop an Active variant of ResourceQueue and make it available as possible 
> queue.provider for ForwardDistributionAgentFactory
> ---
>
> Key: SLING-8853
> URL: https://issues.apache.org/jira/browse/SLING-8853
> Project: Sling
>  Issue Type: Improvement
>  Components: Content Distribution
>Reporter: Ashish Chopra
>Assignee: Timothee Maret
>Priority: Major
> Fix For: Content Distribution Core 0.4.2
>
> Attachments: 
> 0001-SLING-8853-Adds-ActiveResourceQueue-and-makes-it-ava.patch, 
> 0003-SLING-8853-SLING-8854-enabling-ActiveResourceQueue-t.patch
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Via SLING-7754, a [JCR Resource-backed 
> queue|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/queue/impl/resource/ResourceQueue.java]
>  [0] implementation was developed for usecases where a queue processor wasn't 
> required, {{ResourceQueue}} implementation is currently {{PASSIVE}}.
> Given above, it is not possible to leverage {{ResourceQueue}} as a 
> [{{queue.provider}} for {{DistributionAgents}} created via 
> {{ForwardDistributionAgentFactory}}|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/deb3d2ae33c4f4678c8503091a9fffdbb141e569/src/main/java/org/apache/sling/distribution/agent/impl/ForwardDistributionAgentFactory.java#L171-L178]
>  [1].
> This issue aims to add an "Active" variant of Resource backed queues and 
> provide it as an option for {{ForwardDistributionAgentFactory}}.
> [0] 
> https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/queue/impl/resource/ResourceQueue.java
> [1] 
> https://github.com/apache/sling-org-apache-sling-distribution-core/blob/deb3d2ae33c4f4678c8503091a9fffdbb141e569/src/main/java/org/apache/sling/distribution/agent/impl/ForwardDistributionAgentFactory.java#L171-L178



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Comment Edited] (SLING-8853) Develop an Active variant of ResourceQueue and make it available as possible queue.provider for ForwardDistributionAgentFactory

[Ashish Chopra (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-8853?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16987743#comment-16987743
 ] 

Ashish Chopra edited comment on SLING-8853 at 12/4/19 11:02 AM:


I've updated the PR for this issue [0] with the tests for ActiveResourceQueue 
and its corresponding provider's updates.

While I [couldn't get the existing Pax Exam based tests to 
run|https://issues.apache.org/jira/browse/SLING-8853?focusedCommentId=16982140=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982140]
 still, I chose to take a different approach and write the test using Mock 
Sling and Mock Sling Oak test-tooling.

[~marett], would request you to provide feedback on [0] and merge if acceptable.

[0] https://github.com/apache/sling-org-apache-sling-distribution-core/pull/24


was (Author: ashishc):
I've updated the PR for this issue [0] with the tests for ActiveResourceQueue 
and its corresponding provider's updates.

While I [couldn't get the existing Pax Exam based tests to 
run|https://issues.apache.org/jira/browse/SLING-8853?focusedCommentId=16982140=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16982140]
 still, I chose to take a different approach and write the test using Mock 
Sling and Mock Sling Oak test-tooling.

[~marett], would request you to provide feedback on [0] and merge.

[0] https://github.com/apache/sling-org-apache-sling-distribution-core/pull/24

> Develop an Active variant of ResourceQueue and make it available as possible 
> queue.provider for ForwardDistributionAgentFactory
> ---
>
> Key: SLING-8853
> URL: https://issues.apache.org/jira/browse/SLING-8853
> Project: Sling
>  Issue Type: Improvement
>  Components: Content Distribution
>Reporter: Ashish Chopra
>Assignee: Timothee Maret
>Priority: Major
> Fix For: Content Distribution Core 0.4.2
>
> Attachments: 
> 0001-SLING-8853-Adds-ActiveResourceQueue-and-makes-it-ava.patch, 
> 0003-SLING-8853-SLING-8854-enabling-ActiveResourceQueue-t.patch
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Via SLING-7754, a [JCR Resource-backed 
> queue|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/queue/impl/resource/ResourceQueue.java]
>  [0] implementation was developed for usecases where a queue processor wasn't 
> required, {{ResourceQueue}} implementation is currently {{PASSIVE}}.
> Given above, it is not possible to leverage {{ResourceQueue}} as a 
> [{{queue.provider}} for {{DistributionAgents}} created via 
> {{ForwardDistributionAgentFactory}}|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/deb3d2ae33c4f4678c8503091a9fffdbb141e569/src/main/java/org/apache/sling/distribution/agent/impl/ForwardDistributionAgentFactory.java#L171-L178]
>  [1].
> This issue aims to add an "Active" variant of Resource backed queues and 
> provide it as an option for {{ForwardDistributionAgentFactory}}.
> [0] 
> https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/queue/impl/resource/ResourceQueue.java
> [1] 
> https://github.com/apache/sling-org-apache-sling-distribution-core/blob/deb3d2ae33c4f4678c8503091a9fffdbb141e569/src/main/java/org/apache/sling/distribution/agent/impl/ForwardDistributionAgentFactory.java#L171-L178



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Re: [VOTE] Release Apache Sling Content Distribution Journal Core 0.1.6

[Radu Cotescu]

+1

> On 3 Dec 2019, at 21:29, Timothee Maret  wrote:
> 
> Please vote to approve this release:
> 
>  [ ] +1 Approve the release
>  [ ]  0 Don't care
>  [ ] -1 Don't release, because ...

Re: sling.servlet.resolution = strict ?

[Radu Cotescu]

+1

> On 4 Dec 2019, at 11:00, Carsten Ziegeler  wrote:
> 
> And yes, we should rely on capabilities. The servlets resolver bundle already 
> provides such a capability, so we can build on that one.
> If developers use the annotations, we can use the bnd functionality to 
> automatically generate the requirement if the annotation using the new 
> functionality is used.

Re: sling.servlet.resolution = strict ?

[Carsten Ziegeler]

With compatibility I mean that existing code(servlets) work as before, 
even if they specified the extensions/methods properties that are 
ignored today.

Bertrand's proposal covers that.

And yes, we should rely on capabilities. The servlets resolver bundle 
already provides such a capability, so we can build on that one.
If developers use the annotations, we can use the bnd functionality to 
automatically generate the requirement if the annotation using the new 
functionality is used.


Regards
Carsten

Am 04.12.2019 um 10:33 schrieb Konrad Windszus:

Well, compatibility is hard to achieve here.
Imagine you have a servlet registered to a path with an extension.
That obviously requires the new servlet resolver bundle (evaluating the 
sling.servlet.resolution property). Therefore it cannot be backwards 
compatible. It should fail in case you try to run that servlet with an older 
resolver. Therefore I suggest relying on OSGi capabilities here.

Konrad


On 4. Dec 2019, at 10:30, Carsten Ziegeler  wrote:

It's important that we are compatible. For the new property I think we should 
support two values, one of them being the default (works as today) and one for 
the new behaviour. Not specifying the prop then is the same as specifying the 
default value.

I'm not sure if "strict" is a good choice, its not directly visible what it 
means

Regards
Carsten

Am 03.12.2019 um 15:54 schrieb Bertrand Delacretaz:

Hi,
For SLING-8110 I'd like to take into account the
sling.servlet.extensions and sling.servlet.methods properties, if
present, on servlets that are mounted using the sling.servlet.paths
property.
However that's not backwards compatible, as currently these properties
are silently ignored if sling.servlet.paths is present (*)
To keep backwards compatibility, I suggest adding a new optional
sling.servlet.resolution property that for now can have the value
"strict".
If that's present, the new SLING-8110 code takes into account the
sling.servlet.extensions and sling.servlet.methods properties, if
present, for a servlet that has a sling.servlet.paths property.
If sling.servlet.resolution is not set, the resolution behaves as it does today.
WDYT?
-Bertrand
(*) As Konrad notes at SLING-8110 the latest annotations make it
harder to have such ignored properties, but technically nothing
prevents them from being present


--
--
Carsten Ziegeler
Adobe Research Switzerland
cziege...@apache.org




--
--
Carsten Ziegeler
Adobe Research Switzerland
cziege...@apache.org

Re: sling.servlet.resolution = strict ?

[Konrad Windszus]

Well, compatibility is hard to achieve here.
Imagine you have a servlet registered to a path with an extension.
That obviously requires the new servlet resolver bundle (evaluating the 
sling.servlet.resolution property). Therefore it cannot be backwards 
compatible. It should fail in case you try to run that servlet with an older 
resolver. Therefore I suggest relying on OSGi capabilities here.

Konrad

> On 4. Dec 2019, at 10:30, Carsten Ziegeler  wrote:
> 
> It's important that we are compatible. For the new property I think we should 
> support two values, one of them being the default (works as today) and one 
> for the new behaviour. Not specifying the prop then is the same as specifying 
> the default value.
> 
> I'm not sure if "strict" is a good choice, its not directly visible what it 
> means
> 
> Regards
> Carsten
> 
> Am 03.12.2019 um 15:54 schrieb Bertrand Delacretaz:
>> Hi,
>> For SLING-8110 I'd like to take into account the
>> sling.servlet.extensions and sling.servlet.methods properties, if
>> present, on servlets that are mounted using the sling.servlet.paths
>> property.
>> However that's not backwards compatible, as currently these properties
>> are silently ignored if sling.servlet.paths is present (*)
>> To keep backwards compatibility, I suggest adding a new optional
>> sling.servlet.resolution property that for now can have the value
>> "strict".
>> If that's present, the new SLING-8110 code takes into account the
>> sling.servlet.extensions and sling.servlet.methods properties, if
>> present, for a servlet that has a sling.servlet.paths property.
>> If sling.servlet.resolution is not set, the resolution behaves as it does 
>> today.
>> WDYT?
>> -Bertrand
>> (*) As Konrad notes at SLING-8110 the latest annotations make it
>> harder to have such ignored properties, but technically nothing
>> prevents them from being present
> 
> -- 
> --
> Carsten Ziegeler
> Adobe Research Switzerland
> cziege...@apache.org

Re: sling.servlet.resolution = strict ?

[Carsten Ziegeler]

It's important that we are compatible. For the new property I think we 
should support two values, one of them being the default (works as 
today) and one for the new behaviour. Not specifying the prop then is 
the same as specifying the default value.


I'm not sure if "strict" is a good choice, its not directly visible what 
it means


Regards
Carsten

Am 03.12.2019 um 15:54 schrieb Bertrand Delacretaz:

Hi,

For SLING-8110 I'd like to take into account the
sling.servlet.extensions and sling.servlet.methods properties, if
present, on servlets that are mounted using the sling.servlet.paths
property.

However that's not backwards compatible, as currently these properties
are silently ignored if sling.servlet.paths is present (*)

To keep backwards compatibility, I suggest adding a new optional
sling.servlet.resolution property that for now can have the value
"strict".

If that's present, the new SLING-8110 code takes into account the
sling.servlet.extensions and sling.servlet.methods properties, if
present, for a servlet that has a sling.servlet.paths property.

If sling.servlet.resolution is not set, the resolution behaves as it does today.

WDYT?

-Bertrand

(*) As Konrad notes at SLING-8110 the latest annotations make it
harder to have such ignored properties, but technically nothing
prevents them from being present



--
--
Carsten Ziegeler
Adobe Research Switzerland
cziege...@apache.org

[jira] [Assigned] (SLING-8869) SimpleHttpDistributionTransport does not refresh the secret for token based implementations.

[Timothee Maret (Jira)]

 [ 
https://issues.apache.org/jira/browse/SLING-8869?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Timothee Maret reassigned SLING-8869:
-

Assignee: Timothee Maret

> SimpleHttpDistributionTransport does not refresh the secret for token based 
> implementations.
> 
>
> Key: SLING-8869
> URL: https://issues.apache.org/jira/browse/SLING-8869
> Project: Sling
>  Issue Type: Bug
>  Components: Content Distribution
>Reporter: Mohit Arora
>Assignee: Timothee Maret
>Priority: Critical
> Fix For: Content Distribution Core 0.4.2
>
> Attachments: SLING-8869-new.patch, SLING-8869.patch
>
>
> While saving the {{contextKeyExecutor}} in {{DistributionTransportContext}} 
> map, it is not expected that the secret associated with the executor could be 
> expired. This can happen in case of access token based implementations where 
> the token is expired after a certain period of time and has to be refreshed.
> The code to refresh the token is written in the secret provider but since the 
> executor is [cached in the 
> map|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/transport/impl/SimpleHttpDistributionTransport.java#L208]
>  the secrets are not refreshed. It works fine for credentials based secret 
> provider but not for access token based.
> cc - [~marett]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Re: Sling preventing Login

[Robert Munteanu]

Hi Andy,

On Tue, 2019-12-03 at 16:46 -0800, Andreas Schaefer wrote:
> Hi
> 
> I am running into a problem with Sling 11 on Java 8. When I launch
> Sling 11 and then install Peregrine on it everything works just fine.
> But then I restart Sling I am not able to login anymore as the login
> page will tell me that Sling is starting up. In the logs I discovered
> an OSGi cyclic dependencies which makes a health check fail and with
> it I cannot login into Sling anymore.
> 
> Unfortunately I cannot force the login (/system/sling/form/login) as
> this does redirect back to the landing page.
> 
> Is there a way to at least get access to the OSGi console?

Have you tried forcing basic authentication, e.g. entering

  http://admin:admin@localhost:8080/system/console

in the browser location bar?

I think that worked for me.

Additionally, does the problem still exist with the latest Sling
SNAPSHOT? If it does, please file a bug.

Thanks!
Robert

[jira] [Commented] (SLING-8869) SimpleHttpDistributionTransport does not refresh the secret for token based implementations.

[Mohit Arora (Jira)]

[ 
https://issues.apache.org/jira/browse/SLING-8869?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16987641#comment-16987641
 ] 

Mohit Arora commented on SLING-8869:


[~bdelacretaz], [~ashishc] I have attached another patch,  
[^SLING-8869-new.patch]  which keeps the caching as it is and always inject a 
header in request if the credentials provider is authorization based 
credentials provider. As Ashish mentioned, I do not see a clear advantage of 
one approach on the other so we can choose either one.

> SimpleHttpDistributionTransport does not refresh the secret for token based 
> implementations.
> 
>
> Key: SLING-8869
> URL: https://issues.apache.org/jira/browse/SLING-8869
> Project: Sling
>  Issue Type: Bug
>  Components: Content Distribution
>Reporter: Mohit Arora
>Priority: Critical
> Fix For: Content Distribution Core 0.4.2
>
> Attachments: SLING-8869-new.patch, SLING-8869.patch
>
>
> While saving the {{contextKeyExecutor}} in {{DistributionTransportContext}} 
> map, it is not expected that the secret associated with the executor could be 
> expired. This can happen in case of access token based implementations where 
> the token is expired after a certain period of time and has to be refreshed.
> The code to refresh the token is written in the secret provider but since the 
> executor is [cached in the 
> map|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/transport/impl/SimpleHttpDistributionTransport.java#L208]
>  the secrets are not refreshed. It works fine for credentials based secret 
> provider but not for access token based.
> cc - [~marett]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Updated] (SLING-8869) SimpleHttpDistributionTransport does not refresh the secret for token based implementations.

[Mohit Arora (Jira)]

 [ 
https://issues.apache.org/jira/browse/SLING-8869?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Mohit Arora updated SLING-8869:
---
Attachment: SLING-8869-new.patch

> SimpleHttpDistributionTransport does not refresh the secret for token based 
> implementations.
> 
>
> Key: SLING-8869
> URL: https://issues.apache.org/jira/browse/SLING-8869
> Project: Sling
>  Issue Type: Bug
>  Components: Content Distribution
>Reporter: Mohit Arora
>Priority: Critical
> Fix For: Content Distribution Core 0.4.2
>
> Attachments: SLING-8869-new.patch, SLING-8869.patch
>
>
> While saving the {{contextKeyExecutor}} in {{DistributionTransportContext}} 
> map, it is not expected that the secret associated with the executor could be 
> expired. This can happen in case of access token based implementations where 
> the token is expired after a certain period of time and has to be refreshed.
> The code to refresh the token is written in the secret provider but since the 
> executor is [cached in the 
> map|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/transport/impl/SimpleHttpDistributionTransport.java#L208]
>  the secrets are not refreshed. It works fine for credentials based secret 
> provider but not for access token based.
> cc - [~marett]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)