search

framework-one / fw1

FW/1 - Framework One - is a lightweight, convention over configuration, MVC application framework for ColdFusion / CFML.
http://framework-one.github.io
Other
374 stars 141 forks source link

Bad img src causes controller function to be called twice, second time with garbage parameters #508

Closed EverybodyGetsOn closed 5 years ago

EverybodyGetsOn commented 5 years ago

Have a link in my view that looks like this:

<a href="#buildURL(action='employer.JobsAll',queryString='#rc.orderByURL#?Order=1')#">
    Job Title <img src="Images/arrow_Up.gif" alt="" width="7" height="4" />
</a>

As it turns out, that "Images/arrow_Up.gif" was a bad path. The problem is that instead of failing gracefully, this caused FW1 to load my controller function for this page twice, and with some garbage parameters the second time.

Attached image shows a partial dump of my "rc" request context variable from a writedump at the end of the controller. Of note:

  1. It runs twice
  2. The "SelectAll" parameter has been set to "Images"
  3. There is a new parameter called "arrow_Up.gif"

image

Don't know if this is to be expected, or not. It took me days to find the cause, and I'd at least like to give a heads-up so nobody else falls into the same trap.

aliaspooryorik commented 5 years ago

FW/1 will just render the link in the view as HTML mark up - it won't make the controller run twice that I can think of. My guess is that you have some junk in your rc. orderByURL variable (can't see it in your dump), so that when the page is rendered to the browser you have broken URL. If you view the HTML source it may shed some more light on the cause.

EverybodyGetsOn commented 5 years ago

This is what the link looks like when rendered: image Looks like it's got some mixed SES/non-SES in there, so the queryString part of the buildURL is probably wrong, but I even went back and corrected that -to no effect.

Regardless, the controller does run twice, the OrderByURL is a nice clean "Status=1&StartRow=1" both times, and removing the image tag with the bad src fixes the issue. This leads me to believe that the bad src on the image really is the root cause -not to mention the parts of the image src showing up as params in rc on the second controller call.

If nobody can reproduce this issue, all the better. I'm just trying to help out the next guy.

dnando commented 5 years ago

Why does the querystring parameter of buildurl() have a question mark in it rather than an ampersand ?

queryString='#rc.orderByURL#?Order=1')#

That's a genuine question. I see that the fw1 docs sometimes have a question mark in the queryString parameter. I'm wondering if that's a mistake in the docs.

On Thu, Apr 25, 2019 at 5:25 PM EverybodyGetsOn notifications@github.com wrote:

This is what the link looks like when rendered: [image: image] https://user-images.githubusercontent.com/16788368/56746912-b5604b00-6742-11e9-8c4d-c6203b8c29e4.png Looks like it's got some mixed SES/non-SES in there, so the queryString part of the buildURL is probably wrong, but I even went back and corrected that -to no effect.

Regardless, the controller does run twice, the OrderByURL is a nice clean "Status=1&StartRow=1" both times, and removing the image tag with the bad src fixes the issue. This leads me to believe that the bad src on the image really is the root cause -not to mention the parts of the image src showing up as params in rc on the second controller call.

If nobody can reproduce this issue, all the better. I'm just trying to help out the next guy.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/framework-one/fw1/issues/508#issuecomment-486721014, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEHNH2RUSNDE6JQU5ZWC7TPSHEQNANCNFSM4HIN6WOA .

EverybodyGetsOn commented 5 years ago

@dnando I don't honestly know, as I am correcting existing code, but it didn't end up being relevant to the double controller call issue. Like I said in my last post, I went back and corrected it with an ampersand and it made a nice clean SES url, so I think that the ampersand is correct.

dnando commented 5 years ago

Ok, so I think the Reference Manual docs for buildUrl() should be corrected then. As I said, there are a few examples, namely

buildURL( action = ‘product.detail’, queryString = ‘id=42?img=large##overview’ )

buildURL( ‘product.detail?id=42?img=large##overview’ )

that seem to be incorrect, with question marks instead of ampersands.

On Fri, Apr 26, 2019 at 12:20 AM EverybodyGetsOn notifications@github.com wrote:

@dnando https://github.com/dnando I don't honestly know, as I am correcting existing code, but it didn't end up being relevant to the double controller call issue. Like I said in my last post, I went back and corrected it with an ampersand and it made a nice clean SES url, so I think that the ampersand is correct.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/framework-one/fw1/issues/508#issuecomment-486858521, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEHNHYFWXFG47LVMFL4KGTPSIVDRANCNFSM4HIN6WOA .

sneiland commented 5 years ago

@dnando I have correct the reference manual in the wiki.

seancorfield commented 5 years ago

The wiki is ancient documentation and not applicable to any recent versions of the framework.

The documentation is in the framework-one.github.io repo and is built using Jekyll.

If you read the Reference Manual for buildURL() it is correct:

http://framework-one.github.io/documentation/4.2/reference-manual.html#public-string-function-buildurl-string-action---string-path--see-below-any-querystring---

In particular:

buildURL( action = ‘product.detail’, queryString = ‘id=42?img=large##overview’ )

Will generate:

buildURL( ‘product.detail?id=42?img=large##overview’ )

Will also generate:

The important point here is that the ? delineates the shift from SES URL mode to standard query string mode.

In the single argument version of the call, the first ? just delineates the point at which to break the string so the two argument version can effectively be applied.

buildURL( 'foo.bar?a=1&b=2?c=3&d=4' )

will be treated the same as

buildURL( action = 'foo.bar?a=1&b=2', queryString = 'c=3&d=4' )

and produce /foo/bar/a/1/b/2?c=3&d=4 (depending on settings).

So a) revert the wiki text -- it's correct b) ignore the wiki in future as it's only for very old versions of the framework :)

seancorfield commented 5 years ago

The reason your controller runs twice is because the incorrect relative link is resolved by the browser relative to the SES URL which looks like path info, so it ends up being:

/employ/jobsall/status/1/startrow/1/Images/Arrow_up.gif

and I'm guessing that you have some URL rewrite in place that inserts selectall into the URL, so instead of it being zero (or, more likely, an empty string that you param to zero?), you end up with this URL:

/employ/jobsall/status/1/startrow/1/selectall/Images/Arrow_up.gif

which is parsed as

/employ/jobsall?status=1&startrow=1&selectall=Images&Arrow_up.gif=
seancorfield commented 5 years ago

Bottom line: there's no bug in the framework, nor in the documentation. It was a (subtle) error in the OP's code and no action is needed (beyond closing this issue).