Patch for Error accessing dashboard Illegal target of jump or branch… #294

Merged
merged 2 commits into from Sep 16, 2015

Projects

None yet

2 participants

@danlance
Contributor

Current Master branch running on CF9 on Windows 2008 R2 gives Error accessing dashboard Illegal target of jump or branch when attempting to access Taffy dashboard.

This issue is observable running CF9.0.1 CHF 1 on Java 1.6 on Windows 2008 R2, and is still observable running CF9.0.1 CHF4 on Java 1.7.

This issue is not observed on CF9.0.1 running on OSX.

Replacing the continue; statement within the <cfscript> block with <cfcontinue> resolves this issue.

As <cfontinue> is not supported on CF8, this statement has been wrapped in a conditional which will attempt to execute <cfontinue> on CF >= 9, but will use the continue; statement within the <cfscript> block on CF8.

This has only been tested on CF9 on windows and OSX - where it works without issue.
It has not been tested on CF8 as I do not have access to a CF8 environment for testing.

@atuttle
Owner
atuttle commented Sep 16, 2015

Hi Dan, thanks for your effort!

I'm curious which update of Java 7 you're experiencing this on. It's come up in the past (#248), but as people keep running into it, and the fix is fairly easy, I'm not opposed to making a change so that people don't keep running into something that's preventable.

If we're going to rewrite it though, why stick with a continue mechanism at all? (And indeed, having to do both <cfcontinue> and <cfscript>continue;</cfscript> to maintain platform support "smells" a bit to me...)

Instead, what if we rewrite that conditional to (this is untested, just extemporaneous):

<cfif listFindNoCase('get,post,put,delete,patch',local.func.name) OR structKeyExists(local.func,'taffy_verb') OR structKeyExists(local.func,'taffy:verb')>

And then all of the code that currently comes after the continue mechanism would go inside the conditional. I think I originally wrote it with the continue because it saves yet-another-level-of-indentation (which I generally believe to be cleaner code), but clearly it's not serving us well.

Thoughts?

@danlance danlance Refactored dashboard to remove continue; / <cfcontinue>
Resolves Illegal target of jump or branch issue running on CF 9.0.1 on Windows.
0dec920
@danlance
Contributor

OK - agree with you, bit of a code smell with that conditional...

I think the code is just as clear without the continue (possibly more clear to most CF developers as <cfcontinue> is probably not a particularly well known command).

I've refactored and updated the pull request as per your suggestion - and it certainly seems to work fine on CF9 on Windows.

I have encountered this issue with jdk1.7.0_75 (and with jdk1.6.0_24) - but only on Windows...

I have another client running jdk1.6.0_32 on windows 2008 R2 for which Taffy dashboard is functioning without any issues...

Client who is experiencing issues is running a multi server installation of CF9 - whereas one without issues is running a standalone installation... which is probably irelavant - although I know from personal experience that there are a number of inconsistencies and issues which don't occur when running standalone install which do with multiserver... I think they ran out of testing budget at some point!

I suppose it is possible that an issue may have been resolved within jdk1.7.0_55... which was then reintroduced in later versions - and I realise update 75 isn't certified for CF9 (released after Abobe ceased support for CF9)... but it seems unlikely...

As you say - fairly simple fix - and if it prevents this issue from stumbling anyone else in the future, then I believe it's worth doing (and means I don't need to run my client's site on a forked version of Taffy... which makes updates and deployments easier for me :).

@atuttle
Owner
atuttle commented Sep 16, 2015

Awesome, thanks! It'll be nice to finally put this one to bed once and for all.

@atuttle atuttle merged commit 771a26c into atuttle:master Sep 16, 2015
@atuttle atuttle added this to the Taffy 3.1 milestone Sep 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment