Default Project CR-8146

Fix for OPENAM-6196 - don't call getOutputStream() when getWriter() has already been called on the...

Closed on 27 Sep 15

  •  
  •  
  •  
  •  
  • Author & Moderator
  • Reviewers
    • Reviewer completed
    • Reviewer completed
    • Reviewer completed

CR-8146 11

Keyboard shortcuts  
Summarize the review outcomes (optional)
 
#permalink

Details

Warning: no files are visible, they have all been filtered.
Participant Role Time Spent Comments Latest Comment
Author & Moderator 54m 4 Bump
Reviewer - Complete 30m    
Reviewer - Complete 3m    
Reviewer - 55% reviewed 9m 1 Thanks Peter. Added Ken who will be merging in the Util c...
Reviewer - Complete 30m 3 IDPSSOFederate will require a manual merge into our branc...
Total   2h 6m 11  
#permalink

Objectives

As per the JavaDoc - https://docs.oracle.com/javaee/6/api/javax/servlet/ServletResponse.html#getOutputStream()
"Either this method or getWriter() may be called to write the body, not both."

The changes in this review are to, where possible, make use of the PrintWriter provided further upstream by a JSP. There were a couple of checkstyle type improvements (long lines, private constructors on util classes) made at the same time.

Most of required changes happened as part of AME-2227 and back-ported in OPENAM-2327, this review covers a couple of cases that were missed such that calling getOutputStream results in a "java.lang.IllegalStateException: getWriter() has already been called for this response".

When looking for candidates in the federation package, the following were looked at but not changed as they were either a Servlet or only called from a Servlet:

AssertionIDRequestServiceSOAP - a Servlet
AssertionIDRequestUtil - only called from a Servlet
AttributeServiceSOAP - a Servlet
AuthnQueryServiceSOAP - a Servlet
CDCServlet - a Servlet
DoManageNameID - only called from a Servlet
IDPArtifactResolution - only called from a Servlet
IDPSingleLogoutServiceSOAP - a Servlet but also called from a JSP, included in CR
NameIDMappingServiceSOAP - a Servlet
QueryHandlerServlet - a Servlet
SAMLSOAPReceiver - a Servlet
SOAPReceiver - a Servlet
SPSingleLogoutServiceSOAP - a Servlet
SPSSOFederate - only called from a Servlet
WSPRedirectHandlerServlet - a Servlet

FSPreLogin - only called from a Servlet
FSSOAPReceiver - a Servlet
FSSSOAndFedService - Servlet
FSSSOLECPProfileHandler - only called from a Servlet

#permalink

Issues Raised From Comments

Key Summary State Assignee
#permalink

General Comments

08 Sep 15

Peter Major says:

Added AndyF to the review as he's been looking at this in the past, and David so that somebody from Team Newton is aware of these SAML changes.

08 Sep 15

David Luna says:

Thanks Peter. Added Ken who will be merging in the Util changes also.

08 Sep 15

Ken Stubbings says:

IDPSSOFederate will require a manual merge into our branch but it should be very easy - we only refer to sendError once now.

09 Sep 15

markdr says:

Remembered overnight that these changes would also impact some JSP pages, updated to include JSPs that needed updating.

23 Sep 15

markdr says:

Bump

/openam-federation/.../profile/IDPProxyUtil.java Changed  
Open in IDE #permalink
/openam-federation/.../profile/IDPSSOFederate.java Changed   4
Open in IDE #permalink
/openam-federation/.../profile/IDPSSOUtil.java Changed  
Open in IDE #permalink
/openam-federation/.../profile/IDPSingleLogout.java Changed  
Open in IDE #permalink
/openam-federation/.../profile/SPSingleLogout.java Changed   1
Open in IDE #permalink
/openam-federation/.../servlet/IDPSingleLogoutServiceSOAP.java Changed  
Open in IDE #permalink
/openam-server-only/.../jsp/idpSSOFederate.jsp Changed   1
Open in IDE #permalink
/openam-server-only/src/.../jsp/idpSSOInit.jsp Changed  
Open in IDE #permalink
/openam-server-only/.../jsp/spAssertionConsumer.jsp Changed  
Open in IDE #permalink

Review updated: Reload | Ignore | Collapse

You cannot reload the review while writing a comment.

Create Issue

X
Assign To Me

Log time against