Issue Details (XML | Word | Printable)

Key: JAVASERVERFACES-2067
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: rogerk
Reporter: xj
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
javaserverfaces

RI checks context parameters only with lower case "true"

Created: 18/May/11 11:55 PM   Updated: 11/Feb/14 03:43 PM   Resolved: 20/Feb/12 05:13 PM
Component/s: None
Affects Version/s: 2.1.0
Fix Version/s: 2.1.2, 2.2.0-m01

Time Tracking:
Not Specified

File Attachments: 1. Text File changebundle-2067-1.txt (26 kB) 23/May/11 01:37 PM - rogerk
2. Text File changebundle-2067-2.txt (26 kB) 23/May/11 01:55 PM - rogerk
3. Text File changebundle-2067.txt (26 kB) 23/May/11 11:16 AM - rogerk

Issue Links:
Related
 

Tags:
Participants: ioss, rogerk and xj


 Description  « Hide

This is reported against Mojarra-2.1.0.

In 11.1.3 Application Configuration Parameters of JSF 2.0 specification, it says that context parameters of boolean type should be evaluated with toLowerCase().equals("true"). RI only uses lower case "true" to check, is it correct?



rogerk added a comment - 19/May/11 08:50 AM

Yes I think this ok. I don't think the spec is implying that an implementation has to "literally" use toLowerCase().equals("true"). The fact that Mojarra is set up so as to enforce that the boolean values match either true|false is fine.
The way Mojarra does it is better for performance.
Perhaps there could be a spec clarification on this?


xj added a comment - 20/May/11 12:02 AM

Please let me clarify this issue more clearly. We think the implementation should check value regardless of upper case or lower case.

com.sun.faces.config.WebConfiguration#isValueValid:
----->
if (!ALLOWABLE_BOOLEANS.matcher(value).matches()) {
if (LOGGER.isLoggable(Level.WARNING)) {
LOGGER.log(Level.WARNING,
"jsf.config.webconfig.boolconfig.invalidvalue",
new Object[]{ value, param.getQualifiedName(), "true|false" });
}
return false;
}

return true;
<-----

The above source code will return true if the "value" is "true".
However, ALLOWABLE_BOOLEANS.matcher("True").matches() will give a "false" result. This let the following code select default value.

WebConfiguration#processBooleanParameters
----->
if (alternate != null) {
if (isValueValid(param, strValue)) { value = Boolean.valueOf(strValue); } else { value = param.getDefaultValue(); }
<-----


rogerk added a comment - 20/May/11 02:26 PM

So - if I am understanding you correctly - you are saying that in web.xml for example, valid values could be:

<context-param>
<param-name>com.sun.faces.validateXml</param-name>
<param-value>true</param-value>
</context-param>

or

<context-param>
<param-name>com.sun.faces.validateXml</param-name>
<param-value>=True</param-value>
</context-param>

same for "false" or "False",..

If so, what does that buy you? true|false are consistent with java boolean values...

Am I missing somehting here?


rogerk added a comment - 20/May/11 02:32 PM

Never mind - I think I see your point w/r/t the specification.
I'll take a look..


rogerk added a comment - 23/May/11 11:15 AM

Reopening.


rogerk added a comment - 23/May/11 11:16 AM

changes.


ioss added a comment - 23/May/11 01:18 PM

I suggest using:

Pattern.compile("true|false", Pattern.CASE_INSENSITIVE)

to also match for FAlse or truE (as required by the specification)
The testcase should better test for a stranger input like "FalSe" instead of "False".


rogerk added a comment - 23/May/11 01:34 PM

Revised change bundle on the way..


rogerk added a comment - 23/May/11 01:37 PM

Revised change bundle.


rogerk added a comment - 23/May/11 01:55 PM

Revised change bundle (again)...


ioss added a comment - 23/May/11 02:00 PM

r=ioss


rogerk added a comment - 23/May/11 02:04 PM

Committed to trunk:
Sending jsf-ri/src/main/java/com/sun/faces/config/WebConfiguration.java
Adding jsf-test/JAVASERVERFACES-2067
Adding jsf-test/JAVASERVERFACES-2067/build.xml
Adding jsf-test/JAVASERVERFACES-2067/htmlunit
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/pom.xml
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java/com
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java/com/sun
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java/com/sun/faces
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java/com/sun/faces/systest
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java/com/sun/faces/systest/Issue2067TestCase.java
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/pom.xml
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src/main
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src/main/webapp
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src/main/webapp/WEB-INF
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src/main/webapp/WEB-INF/web.xml
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src/main/webapp/index.xhtml
Sending jsf-test/build.xml
Transmitting file data ........
Committed revision 9079.


rogerk added a comment - 23/May/11 02:05 PM

Committed. Will commit to 2.1.X branch as well.


rogerk added a comment - 23/May/11 02:27 PM

Committed to MOJARRA_2_1_X_ROLLING branch:
Sending jsf-ri/src/main/java/com/sun/faces/config/WebConfiguration.java
Adding jsf-test/JAVASERVERFACES-2067
Adding jsf-test/JAVASERVERFACES-2067/build.xml
Adding jsf-test/JAVASERVERFACES-2067/htmlunit
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/pom.xml
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java/com
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java/com/sun
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java/com/sun/faces
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java/com/sun/faces/systest
Adding jsf-test/JAVASERVERFACES-2067/htmlunit/src/main/java/com/sun/faces/systest/Issue2067TestCase.java
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/pom.xml
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src/main
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src/main/webapp
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src/main/webapp/WEB-INF
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src/main/webapp/WEB-INF/web.xml
Adding jsf-test/JAVASERVERFACES-2067/i_jsf_2067/src/main/webapp/index.xhtml
Sending jsf-test/build.xml
Transmitting file data ........
Committed revision 9080.

Change bundle is the same as trunk except for jsf-test/build.xml due to the fact that there were fewer tests.


rogerk added a comment - 27/May/11 09:16 AM

reopen to edit fix version


rogerk added a comment - 27/May/11 09:17 AM

fix version


rogerk added a comment - 27/May/11 09:17 AM

re-closing