getFullyQualifiedAction() includes subsystem delimiter for default action #379

Closed
msp1kpj opened this Issue Sep 30, 2015 · 6 comments

Comments

Projects
None yet
3 participants
@msp1kpj

msp1kpj commented Sep 30, 2015

Hello,

I wrote before about having an issue with the sub-system default action. I think that it is a bug.
Setup:
Here is my framework config
variables.framework = {
diEngine = 'app.framework.ioc'
, diLocations = "/app/model/services,/app/model/gateways,./app/model/helpers"
, base = '/app'
, reloadApplicationOnEveryRequest = this.development
, trace = this.development
, generateSES = true
, SESOmitIndex = true
, cacheFileExists = !this.development
, unhandledPaths = '/api'
};

When I go to http://loalhost/ I expected the "rc.action" to be "main.default" but instead I get ":main.deafult". Since I have built my security model around the rc.action this change is breaking my security.

I am using the getSectionAndItem(":main.default") as a reference of what to expect and it is not the same as the default action.

@Richardtugwell

This comment has been minimized.

Show comment
Hide comment
@Richardtugwell

Richardtugwell Sep 30, 2015

Are you using subsystems? getSectionAndItem should be subsystem agnostic as
far as I can tell from the docs. Why prepend ':' to main.default?

On 30 September 2015 at 18:03, Ken John notifications@github.com wrote:

Hello,

I wrote before about having an issue with the sub-system default action. I
think that it is a bug.
Setup:
Here is my framework config
variables.framework = {
diEngine = 'app.framework.ioc'
, diLocations =
"/app/model/services,/app/model/gateways,./app/model/helpers"
, base = '/app'
, reloadApplicationOnEveryRequest = this.development
, trace = this.development
, generateSES = true
, SESOmitIndex = true
, cacheFileExists = !this.development
, unhandledPaths = '/api'
};

When I go to http://loalhost/ I expected the "rc.action" to be
"main.default" but instead I get ":main.deafult". Since I have built my
security model around the rc.action this change is breaking my security.

I am using the getSectionAndItem(":main.default") as a reference of what
to expect and it is not the same as the default action.


Reply to this email directly or view it on GitHub
#379.

Richard Tugwell
http://blog.richardtugwell.com
r.tugwell@forthmedia.com

Are you using subsystems? getSectionAndItem should be subsystem agnostic as
far as I can tell from the docs. Why prepend ':' to main.default?

On 30 September 2015 at 18:03, Ken John notifications@github.com wrote:

Hello,

I wrote before about having an issue with the sub-system default action. I
think that it is a bug.
Setup:
Here is my framework config
variables.framework = {
diEngine = 'app.framework.ioc'
, diLocations =
"/app/model/services,/app/model/gateways,./app/model/helpers"
, base = '/app'
, reloadApplicationOnEveryRequest = this.development
, trace = this.development
, generateSES = true
, SESOmitIndex = true
, cacheFileExists = !this.development
, unhandledPaths = '/api'
};

When I go to http://loalhost/ I expected the "rc.action" to be
"main.default" but instead I get ":main.deafult". Since I have built my
security model around the rc.action this change is breaking my security.

I am using the getSectionAndItem(":main.default") as a reference of what
to expect and it is not the same as the default action.


Reply to this email directly or view it on GitHub
#379.

Richard Tugwell
http://blog.richardtugwell.com
r.tugwell@forthmedia.com

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Sep 30, 2015

Member

Not a bug. rc.action is only mentioned in one place in the docs (as a way to see which action caused onMissingView() to be called) and if you look at http://framework-one.github.io/documentation/3.5/reference-manual.html#request-scope you'll see this comment about request.action (which is a legacy version of rc.action):

request.action - The action specified in the URL or form (after expansion to the fully qualified form). This can be obtained by calling getSubsystemSectionAndItem(). Depending on the format you want, you may want to call getFullyQualifiedAction() instead, which will only contain the subsystem delimiter if the subsystem name is non-empty. That is the legacy format (for Subsystems 1.0 applications or applications containing no subsystems).

You should use the get* functions instead of accessing rc.action directly.

Member

seancorfield commented Sep 30, 2015

Not a bug. rc.action is only mentioned in one place in the docs (as a way to see which action caused onMissingView() to be called) and if you look at http://framework-one.github.io/documentation/3.5/reference-manual.html#request-scope you'll see this comment about request.action (which is a legacy version of rc.action):

request.action - The action specified in the URL or form (after expansion to the fully qualified form). This can be obtained by calling getSubsystemSectionAndItem(). Depending on the format you want, you may want to call getFullyQualifiedAction() instead, which will only contain the subsystem delimiter if the subsystem name is non-empty. That is the legacy format (for Subsystems 1.0 applications or applications containing no subsystems).

You should use the get* functions instead of accessing rc.action directly.

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Sep 30, 2015

Member

Also note that rc.action only works if your framework.action config is set to "action" so it really should never be used directly in code (and that means that note in the onMissingView() section of the docs is wrong, really).

Member

seancorfield commented Sep 30, 2015

Also note that rc.action only works if your framework.action config is set to "action" so it really should never be used directly in code (and that means that note in the onMissingView() section of the docs is wrong, really).

@msp1kpj

This comment has been minimized.

Show comment
Hide comment
@msp1kpj

msp1kpj Sep 30, 2015

ok, I have updated my code to use getFullyQualifiedAction() and that still returns ":main.default" for http://localhost/ and "main.default" for http://localhost/main/default

msp1kpj commented Sep 30, 2015

ok, I have updated my code to use getFullyQualifiedAction() and that still returns ":main.default" for http://localhost/ and "main.default" for http://localhost/main/default

@seancorfield seancorfield reopened this Sep 30, 2015

@seancorfield seancorfield added blocker and removed notabug labels Sep 30, 2015

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Sep 30, 2015

Member

OK, that definitely should not happen -- and I've reproduced it locally: getFullyQualifiedAction() should never include the subsystem delimiter if there's no subsystem.

Member

seancorfield commented Sep 30, 2015

OK, that definitely should not happen -- and I've reproduced it locally: getFullyQualifiedAction() should never include the subsystem delimiter if there's no subsystem.

@seancorfield seancorfield changed the title from Default Action - With no subsystem to getFullyQualifiedAction() includes subsystem delimiter for default action Sep 30, 2015

@seancorfield seancorfield added this to the 3.5 milestone Sep 30, 2015

@seancorfield seancorfield self-assigned this Sep 30, 2015

seancorfield added a commit that referenced this issue Sep 30, 2015

Fix #379 so subsystem delimiter omitted
There was an edge case -- where the requested action included the
delimiter -- where this method could include the delimiter when it was
not supposed to. I believe it now correctly takes both the request and
the implied current action into account!

Also fixes default for home action (which previously did not call
getFullyQualifiedAction() when it should have!).
@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Sep 30, 2015

Member

This should be fixed in the latest develop download. I guess I'll be doing an RC 2 since folks have run into a number of bugs in RC 1... :)

Member

seancorfield commented Sep 30, 2015

This should be fixed in the latest develop download. I guess I'll be doing an RC 2 since folks have run into a number of bugs in RC 1... :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment