dev@javaserverfaces.java.net

Seeking review: [2973-FlashCorruptCookie]

From: Edward Burns <edward.burns_at_oracle.com>
Date: Tue, 6 Aug 2013 11:47:53 -0700

http://java.net/jira/browse/JAVASERVERFACES-2973 Deal gracefully with cookies that fail to decode


SECTION: Modified Files
----------------------------
M jsf-ri/src/main/java/com/sun/faces/util/ByteArrayGuardAESCTR.java

- change the contract for decrypt() to throw InvalidKeyException.

M jsf-ri/src/main/java/com/sun/faces/context/flash/ELFlash.java

- Handle the case when InvalidKeyException is thrown by logging a
  message and throwing away the corrupt PreviousNextFlashInfoManager and
  getting a new one, as if this were the first time the flash was used
  on this request.

A test/agnostic/flash/basic/src/test/java/com/sun/faces/test/agnostic/flash/issue2973
A test/agnostic/flash/basic/src/test/java/com/sun/faces/test/agnostic/flash/issue2973/Issue2973IT.java
A test/agnostic/flash/basic/src/main/java/com/sun/faces/test/agnostic/flash/issue2973
A test/agnostic/flash/basic/src/main/java/com/sun/faces/test/agnostic/flash/issue2973/Bean.java
A test/agnostic/flash/basic/src/main/webapp/issue2973
A test/agnostic/flash/basic/src/main/webapp/issue2973/page1.xhtml
A test/agnostic/flash/basic/src/main/webapp/issue2973/page2.xhtml

- Test content, taken from
  <http://mkblog.exadel.com/2010/07/learning-jsf2-using-flash-scope/>.

SECTION: Diffs
----------------------------
Index: jsf-ri/src/main/java/com/sun/faces/context/flash/ELFlash.java
===================================================================
--- jsf-ri/src/main/java/com/sun/faces/context/flash/ELFlash.java (revision 12353)
+++ jsf-ri/src/main/java/com/sun/faces/context/flash/ELFlash.java (working copy)
@@ -48,6 +48,7 @@
 import java.io.UnsupportedEncodingException;
 import java.net.URLDecoder;
 import java.net.URLEncoder;
+import java.security.InvalidKeyException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -1059,8 +1060,18 @@
 
         if (null == result) {
             result = new PreviousNextFlashInfoManager(guard, flashInnerMap);
- result.decode(context, this, cookie);
- contextMap.put(CONSTANTS.RequestFlashManager, result);
+ try {
+ result.decode(context, this, cookie);
+ contextMap.put(CONSTANTS.RequestFlashManager, result);
+ } catch (InvalidKeyException ike) {
+ if (LOGGER.isLoggable(Level.SEVERE)) {
+ result = getCurrentFlashManager(contextMap, true);
+ LOGGER.log(Level.SEVERE,
+ "jsf.externalcontext.flash.bad.cookie",
+ new Object [] { ike.getMessage() });
+ }
+
+ }
 
         }
         return result;
@@ -1200,23 +1211,27 @@
          * the system. When any error occurs, the flash is not usable
          * for this request, and a nice error message is logged.</p>
 
- * <p>This method is where the LifetimeMarker is incremeted,
+ * <p>This method is where the LifetimeMarker is incremented,
          * UNLESS the incoming request is the GET after the REDIRECT
          * after POST, in which case we don't increment it because the
          * system will expire the entries in the doLastPhaseActions.</p>
          *
          */
 
- void decode(FacesContext context, ELFlash flash, Cookie cookie) {
+ void decode(FacesContext context, ELFlash flash, Cookie cookie) throws InvalidKeyException {
             String temp;
             String value;
             
+ String urlDecodedValue = null;
+
             try {
- value = guard.decrypt(URLDecoder.decode(cookie.getValue(), "UTF-8"));
+ urlDecodedValue = URLDecoder.decode(cookie.getValue(), "UTF-8");
             } catch (UnsupportedEncodingException uee) {
- value = guard.decrypt(cookie.getValue());
+ urlDecodedValue = cookie.getValue();
             }
             
+ value = guard.decrypt(urlDecodedValue);
+
             try {
                 int i = value.indexOf("_");
 
Index: jsf-ri/src/main/java/com/sun/faces/util/ByteArrayGuardAESCTR.java
===================================================================
--- jsf-ri/src/main/java/com/sun/faces/util/ByteArrayGuardAESCTR.java (revision 12353)
+++ jsf-ri/src/main/java/com/sun/faces/util/ByteArrayGuardAESCTR.java (working copy)
@@ -41,6 +41,9 @@
 package com.sun.faces.util;
 
 import java.nio.charset.Charset;
+import java.security.InvalidAlgorithmParameterException;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
 import javax.crypto.Cipher;
 import javax.crypto.KeyGenerator;
 import javax.crypto.Mac;
@@ -51,6 +54,9 @@
 import java.util.SortedMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.crypto.BadPaddingException;
+import javax.crypto.IllegalBlockSizeException;
+import javax.crypto.NoSuchPaddingException;
 import javax.xml.bind.DatatypeConverter;
 
 /**
@@ -133,7 +139,7 @@
         return securedata;
     }
 
- public String decrypt(String value) {
+ public String decrypt(String value) throws InvalidKeyException {
         
         byte[] bytes = DatatypeConverter.parseBase64Binary(value);;
         
@@ -142,9 +148,22 @@
             decryptCipher.init(Cipher.DECRYPT_MODE, sk, ivspec);
 
             byte[] plaindata = decryptCipher.doFinal(bytes);
+ for (byte cur : plaindata) {
+ if (cur < 0 || cur > 255) {
+ throw new InvalidKeyException("Invalid characters in decrypted value");
+ }
+ }
             return new String(plaindata, utf8);
- } catch (Exception e) {
- throw new FacesException(e);
+ } catch (NoSuchAlgorithmException nsae) {
+ throw new InvalidKeyException(nsae);
+ } catch (NoSuchPaddingException nspe) {
+ throw new InvalidKeyException(nspe);
+ } catch (InvalidAlgorithmParameterException iape) {
+ throw new InvalidKeyException(iape);
+ } catch (IllegalBlockSizeException ibse) {
+ throw new InvalidKeyException(ibse);
+ } catch (BadPaddingException bpe) {
+ throw new InvalidKeyException(bpe);
         }
     }
     
Index: test/agnostic/flash/basic/src/test/java/com/sun/faces/test/agnostic/flash/issue2973/Issue2973IT.java
===================================================================
--- test/agnostic/flash/basic/src/test/java/com/sun/faces/test/agnostic/flash/issue2973/Issue2973IT.java (revision 0)
+++ test/agnostic/flash/basic/src/test/java/com/sun/faces/test/agnostic/flash/issue2973/Issue2973IT.java (revision 0)
@@ -0,0 +1,105 @@
+/*
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
+ *
+ * Copyright (c) 1997-2010 Oracle and/or its affiliates. All rights reserved.
+ *
+ * The contents of this file are subject to the terms of either the GNU
+ * General Public License Version 2 only ("GPL") or the Common Development
+ * and Distribution License("CDDL") (collectively, the "License"). You
+ * may not use this file except in compliance with the License. You can
+ * obtain a copy of the License at
+ * https://glassfish.dev.java.net/public/CDDL+GPL_1_1.html
+ * or packager/legal/LICENSE.txt. See the License for the specific
+ * language governing permissions and limitations under the License.
+ *
+ * When distributing the software, include this License Header Notice in each
+ * file and include the License file at packager/legal/LICENSE.txt.
+ *
+ * GPL Classpath Exception:
+ * Oracle designates this particular file as subject to the "Classpath"
+ * exception as provided by Oracle in the GPL Version 2 section of the License
+ * file that accompanied this code.
+ *
+ * Modifications:
+ * If applicable, add the following below the License Header, with the fields
+ * enclosed by brackets [] replaced by your own identifying information:
+ * "Portions Copyright [year] [name of copyright owner]"
+ *
+ * Contributor(s):
+ * If you wish your version of this file to be governed by only the CDDL or
+ * only the GPL Version 2, indicate your decision by adding "[Contributor]
+ * elects to include this software in this distribution under the [CDDL or GPL
+ * Version 2] license." If you don't indicate a single choice of license, a
+ * recipient has the option to distribute your version of this file under
+ * either the CDDL, the GPL Version 2 or to extend the choice of license to
+ * its licensees as provided above. However, if you add GPL Version 2 code
+ * and therefore, elected the GPL Version 2 license, then the option applies
+ * only if the new code is made subject to such option by the copyright
+ * holder.
+ */
+
+package com.sun.faces.test.agnostic.flash.issue2973;
+
+import com.gargoylesoftware.htmlunit.html.HtmlElement;
+import org.junit.Test;
+import com.gargoylesoftware.htmlunit.WebClient;
+
+
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import com.gargoylesoftware.htmlunit.html.HtmlSubmitInput;
+import com.gargoylesoftware.htmlunit.html.HtmlTextInput;
+import org.junit.After;
+import org.junit.Before;
+
+import static org.junit.Assert.assertEquals;
+
+public class Issue2973IT {
+
+ private String webUrl;
+ private WebClient webClient;
+
+ @Before
+ public void setUp() {
+ webUrl = System.getProperty("integration.url");
+ webClient = new WebClient();
+ }
+
+ @After
+ public void tearDown() {
+ webClient.closeAllWindows();
+ }
+
+
+ // ------------------------------------------------------------ Test Methods
+
+ @Test
+ public void testServerRestartHandledGracefully() throws Exception {
+
+ HtmlPage page = webClient.getPage(webUrl + "faces/issue2973/page1.xhtml") ;
+ webClient.getOptions().setRedirectEnabled(true);
+ HtmlTextInput textInput = (HtmlTextInput) page.getElementById("input");
+ String message = "" + System.currentTimeMillis();
+ textInput.setValueAttribute(message);
+ HtmlSubmitInput button = (HtmlSubmitInput) page.getElementById("button");
+ page = button.click();
+ HtmlElement value = (HtmlElement) page.getElementById("response");
+
+ assertEquals(message, value.asText());
+
+ page = webClient.getPage(webUrl + "faces/issue2973/page1.xhtml") ;
+ button = (HtmlSubmitInput) page.getElementById("restart");
+ page = button.click();
+
+ textInput = (HtmlTextInput) page.getElementById("input");
+ message = "" + System.currentTimeMillis();
+ textInput.setValueAttribute(message);
+ button = (HtmlSubmitInput) page.getElementById("button");
+ page = button.click();
+ value = (HtmlElement) page.getElementById("response");
+
+ assertEquals(message, value.asText());
+
+ }
+
+
+}
Index: test/agnostic/flash/basic/src/main/java/com/sun/faces/test/agnostic/flash/issue2973/Bean.java
===================================================================
--- test/agnostic/flash/basic/src/main/java/com/sun/faces/test/agnostic/flash/issue2973/Bean.java (revision 0)
+++ test/agnostic/flash/basic/src/main/java/com/sun/faces/test/agnostic/flash/issue2973/Bean.java (revision 0)
@@ -0,0 +1,70 @@
+/*
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
+ *
+ * Copyright (c) 1997-2012 Oracle and/or its affiliates. All rights reserved.
+ *
+ * The contents of this file are subject to the terms of either the GNU
+ * General Public License Version 2 only ("GPL") or the Common Development
+ * and Distribution License("CDDL") (collectively, the "License"). You
+ * may not use this file except in compliance with the License. You can
+ * obtain a copy of the License at
+ * https://glassfish.dev.java.net/public/CDDL+GPL_1_1.html
+ * or packager/legal/LICENSE.txt. See the License for the specific
+ * language governing permissions and limitations under the License.
+ *
+ * When distributing the software, include this License Header Notice in each
+ * file and include the License file at packager/legal/LICENSE.txt.
+ *
+ * GPL Classpath Exception:
+ * Oracle designates this particular file as subject to the "Classpath"
+ * exception as provided by Oracle in the GPL Version 2 section of the License
+ * file that accompanied this code.
+ *
+ * Modifications:
+ * If applicable, add the following below the License Header, with the fields
+ * enclosed by brackets [] replaced by your own identifying information:
+ * "Portions Copyright [year] [name of copyright owner]"
+ *
+ * Contributor(s):
+ * If you wish your version of this file to be governed by only the CDDL or
+ * only the GPL Version 2, indicate your decision by adding "[Contributor]
+ * elects to include this software in this distribution under the [CDDL or GPL
+ * Version 2] license." If you don't indicate a single choice of license, a
+ * recipient has the option to distribute your version of this file under
+ * either the CDDL, the GPL Version 2 or to extend the choice of license to
+ * its licensees as provided above. However, if you add GPL Version 2 code
+ * and therefore, elected the GPL Version 2 license, then the option applies
+ * only if the new code is made subject to such option by the copyright
+ * holder.
+
+ */
+package com.sun.faces.test.agnostic.flash.issue2973;
+
+import javax.faces.bean.ManagedBean;
+import javax.faces.context.FacesContext;
+
+_at_ManagedBean(name="bean")
+public class Bean {
+ private String text;
+
+ public String getText() {
+ return text;
+ }
+
+ public void setText(String text) {
+ this.text = text;
+ }
+
+ public String nextpage (){
+ FacesContext.getCurrentInstance().getExternalContext().getFlash().put("bean", this);
+ return "page2?faces-redirect=true";
+ }
+
+ public String simulateServerRestart() {
+ FacesContext context = FacesContext.getCurrentInstance();
+ context.getExternalContext().getApplicationMap().remove("csfcff");
+
+ return null;
+ }
+
+}
Index: test/agnostic/flash/basic/src/main/webapp/issue2973/page1.xhtml
===================================================================
--- test/agnostic/flash/basic/src/main/webapp/issue2973/page1.xhtml (revision 0)
+++ test/agnostic/flash/basic/src/main/webapp/issue2973/page1.xhtml (revision 0)
@@ -0,0 +1,20 @@
+<?xml version='1.0' encoding='UTF-8' ?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml"
+ xmlns:h="http://java.sun.com/jsf/html">
+ <h:head>
+ <title>Facelet Title</title>
+ </h:head>
+ <h:body>
+ <h:form prependId="false">
+ <h:panelGrid>
+ <h:outputText value="Text:" />
+ <h:inputText id="input" value="#{bean.text}" />
+ <h:commandButton id="button" action="#{bean.nextpage}" value="Click" />
+ <h:commandButton id="restart" action="#{bean.simulateServerRestart}" value="Simulate server restart" />
+
+ </h:panelGrid>
+ </h:form>
+ </h:body>
+</html>
+
Index: test/agnostic/flash/basic/src/main/webapp/issue2973/page2.xhtml
===================================================================
--- test/agnostic/flash/basic/src/main/webapp/issue2973/page2.xhtml (revision 0)
+++ test/agnostic/flash/basic/src/main/webapp/issue2973/page2.xhtml (revision 0)
@@ -0,0 +1,12 @@
+<?xml version='1.0' encoding='UTF-8' ?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml"
+ xmlns:h="http://java.sun.com/jsf/html">
+ <h:head>
+ <title>Facelet Title</title>
+ </h:head>
+ <h:body>
+ <p id="response">#{flash.bean.text}</p>
+ </h:body>
+</html>
+


SECTION: New Files
----------------------------
SEE ATTACHMENTS

--