Coding decisions in Lucee

dev
1

@Brad_Wood did raise some questions here [LDEV-3979] - Lucee about decisions we make when it comes to Lucee development. I hope the following helps to clarify things and maybe is the start of a helpful discussion.

his comment was

Yes, this ticket is good to go as far as working.  X seems AWOL, but it’s working so far as I can tell.  

That said, I think it’s perfectly fair to question why Lucee is doing what it’s doing.  
The entire reason we’re having this conversation is because the outcome wasn’t correct and it seems we created problems for ourselves by not just using the servlet’s cookies.  
The old, “Just trust us, we did this for a reason but we didn’t document why and we don’t need to explain it to you” doesn’t sit very well with me.

When we the Lucee Team are working on the Lucee core we have to made decisions all the time, for example I worked today mainly at the new S3 extensions 2.0 and i had to made some decisions in how to do things.
Some decisions i do make by myself and some i do escalate, it always helps to get more opinion and additional eyes on stuff, but that of course takes also time. I did discuss a lot of it with @Zackster, he did help to shape my decisions on this, but i did not escalate any of this decisions to the community, because non would affect them in any way.

Normally we only escalate the decision process to the community, when it also affects the community. For example when we change the interface of a tag or a function, add a new feature, anything that affects the community, but as long it does not affect the community we do not communicate it most of the time. I’m lucky to have a lot of smart people around me that i can ask for any specific topic.

Sure sometime it happens that a change like that leads to a regression, what then affects the community, but you never know it advance.

In that specific example, we saw that if a cookie is set with help of the header tag not following a 100% the pattern for cookies (having a space in the value), Tomcat still could handle it, but Undertow could not (or the other way around). So we decided to parse the cookies ourself, that way we always get the same result, independent of the servlet engines we are using and Lucee is more flexible on inputs. The pattern for cookies actually is very simple. The same way BTW we read the form and URL scope ourself (including multipart), we decided a long time ago no longer to use what is coming from JSP (Yes Lucee is “based” on JSP) for this scopes.
Problem was our solution did only read one “Cookie” header and ignored, when there is more that one. Even the specification only allows one, reality looks different. So because of that the community was affected, but that is the reason we are doing Snapshots and Release candidates so we catch things like this.

I hope this helps to understand how we come to decisions and that our goal is to include the community as much as possible.

If you disagree with me please do, loudly! :wink:

2

Thanks for this.

To be clear, I didn’t complain because the community wasn’t involved in the decision. I complained because it was a decision that certainly cut against the grain of what would be expected and no explanation or documentation existed

  • in a test case
  • in a ticket description/comment
  • or in comments in the code

You should have seen the strange looks I got from the JBoss Undertow core developers when I told them I was using an app server that ignored their servlet cookies in favor of parsing them manually :laughing:

Furthermore, when I pressed for just an explanation of what exactly was being worked around, I couldn’t get a hard example provided. Just vague statements about servlets not handling and edge cases.

I’m no stranger to having to do wonky things to work around bugs in 3rd party libraries, but when I do, I try to

  • file a ticket with that library
  • place a comment in the code explaining why I’m doing something wonky
  • link to the ticket

This only takes a couple minutes and here’s a perfect example in Runwar’s source code where a feature isn’t implemented yet due to an incomplete Undertow Ticket (which we already sent a pull to fix):

Or this example in CFConfig, where I do something oddball, but for a specific reason:

Now, to address your explanation.

Ahh, finally I see what you mean. A manually-set cookie value with a space. Here is the cfheader code from the original ticket which now finally makes sense.

cfheader(name="Set-Cookie", value="test_cfh=Plain space");

Here is the issue-- This is an invalid set-cookie header per the HTTP spec! RFC-2109, RFC-2965, and RFC-6265 all expressly forbid a space to exist in the value of a cookie. The proper version of that code requires the value be double quoted like so:

cfheader(name="Set-Cookie", value='test_cfh="Plain space"');

Or the special character be encoded in some manner that the app will decode later when reading the value (Lucee uses this approach internally).

cfheader(name="Set-Cookie", value='test_cfh=Plain%20space');

And, of course, it should be noted if a user uses the cookie scope, this works fine as Lucee encodes the value automatically.

cookie.test_cfh="Plain space";

So, this is NOT a bug in Tomcat or Undertow-- they are following the spec. Lucee has now made itself incompatible with the HTTP spec in order to facilitate bad CFML code written by developers who don’t know the HTTP spec.

This is why I repeatedly asked for an example of just exactly what Lucee was trying to work around. it turns out, this is not an “edge case” nor a scenario the “servlet engine could not handle”, it’s simply invalid code that never should have worked in the first place. I know your philosophy on being strict about what you output and liberal about what you accept, so if this boils down to Lucee wanting to allow invalid cookie values to still be usable in their application, then that should have been the response when I first asked, and it should be documented somewhere that Lucee’s cookie parsing deviates from all RFCs on the matter.

3

@bdw429s again at least one of the servlet engines does allow that pattern as well, as i think to remember it is tomcat, so following your logic, they do also not follow the specs.
then the regression was caused by having multiple “Cookie” header that is also not following the specs, as you did agree is a greyzone at best. I said greyzone, because the language in the spec is hard to understand. But in stackoverflow they cleary saying, this is breaking the rules. Talking about undertow breaking specs, you know yourself the security holes we encountered with undertow the last year that are clearly “breaking specs”.

Are you aware how many HTTP specs we had to “bend” to make the tag cfhttp work and make it compatible to ACF. You have to allow all kind of crappy stuff and also accept it on server side.
Are you aware how many tickets we got over the years that said “when i do X in ACF it works, but it does not in Lucee/Railo”. Remember ACF started with their own Servlet Engine JRun that was not following the specs by the book at all.

exceeding specification is part f the game!

Sure i could simply rejects tickets like this and answer “this is not following the HTTP specs or the servlet specs or the JDBC specs (don’t get me started on Oracle bending, no breaking the JDBC specs) …”, but then a project migrating from ACF to Lucee would be blocked. The origin ticket was raised by me, because a client of Rasia did report it to us. I cannot remember the details anymore, but i think it was because it was working with ACF. But i will double check.

4

The main takeaway here needs to be that when Lucee does unexpected things, we document the what and the why. Look at these recent CommandBox tickets where I purposefully “broke” the npm-flavor semantic versioning spec.

https://ortussolutions.atlassian.net/browse/COMMANDBOX-1295
https://ortussolutions.atlassian.net/browse/COMMANDBOX-1436

I both cases, I clearly documented

  • what the spec required
  • what change was being made
  • what the use case for the change was
7

Your example involves stuff affecting users, hell even i was affected by this, we did define the Lucee version numbers as it should be in server.json and it then did ignore it and load the latest version and even update the version when there was a new Lucee version available 2 weeks later what really is the worst thing it could do! It really simply should break in this case. I agree this needs documentation, but how internally something is achieved does not need an external documentation. I need to be documented in the code itself, unless the code does speak for itself.

The manual of my car does not tell me, how the BMS of my car work, because it does not affect me as long it works correctly. The same way a user of Lucee does not care how the cookie data are collected as long they are correct.