Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebServiceTemplate doesn't correctly handle fault for a non-500 http status response [SWS-925] #998

Closed
gregturn opened this issue Nov 7, 2015 · 5 comments

Comments

@gregturn
Copy link
Member

@gregturn gregturn commented Nov 7, 2015

Eric Deandrea opened SWS-925 and commented

WebServiceTemplate doesn't correctly handle faults on a non-500 http status response. If you look at the handleFault method in WebServiceTemplate on line 740 - the method returns false if the connection does not have a fault (500 http status). In our scenario, the response comes back as a 200 but inside the soap message there is a fault. Lines 743-746 never get executed, which would have caught the fault in the soap message and would have made the handleFault method return true, which then kicks in the appropriate faultMessageResolver from the doSendAndReceive method (lines 613-621). Because of this the if statement on line 613 returns true and the responseExtractor tries to extract the data but gets Unmarshalling exceptions. Instead in this situation hasFault should return true and therefore the else block (lines 619-621) should kick in.

I've had to create a workaround by extending WebServiceTemplate like this:

public class FixedWebServiceTemplate extends WebServiceTemplate {
public FixedWebServiceTemplate(WebServiceMessageSender messageSender, WebServiceMessageFactory messageFactory, ClientInterceptor interceptor) {
super(messageFactory);

		setMessageSender(messageSender);
		setInterceptors(new ClientInterceptor[] { new CustomClientInterceptor(), interceptor });
		setFaultMessageResolver(new SoapFaultMessageResolver());
	}

	protected boolean hasFault(WebServiceConnection connection, WebServiceMessage response) throws IOException {
		boolean hasFault = super.hasFault(connection, response);

		// Underlying logic is flawed in my opinion - it doesn't handle the case where the response is a 200 but the
		// response payload contains a soap fault
		return (!hasFault && (response instanceof FaultAwareWebServiceMessage)) ? ((FaultAwareWebServiceMessage) response).hasFault() : hasFault;
	}
}

Affects: 2.2.0.RELEASE, 2.2.1, 2.3.0, 3.0.0.RC1, 2.2.2

@gregturn
Copy link
Member Author

@gregturn gregturn commented Nov 16, 2015

Greg Turnquist commented

The SOAP specs dictate that SOAP faults must be sent with a 500 response code (in 1.1) or with a 400 or 500 response code (in 1.2). To handle faults coming from a 200 status code would not be spec compliant.

http://www.w3.org/TR/2000/NOTE-SOAP-20000508/#_Toc478383529

http://www.w3.org/TR/soap12-part2/#tabresstatereccodes

@gregturn
Copy link
Member Author

@gregturn gregturn commented Nov 16, 2015

Eric Deandrea commented

Thanks for the info.

@gregturn
Copy link
Member Author

@gregturn gregturn commented Nov 17, 2015

Arjen Poutsma commented

Eric Deandrea Did you try to disable the checkConnectionForFault property? If this property is disabled, it will bypass the check on the HTTP connection status, and will check the message instead.

@gregturn
Copy link
Member Author

@gregturn gregturn commented Nov 17, 2015

Eric Deandrea commented

I did see that. The issue with that is by setting that it is a mutually-exclusive setting - either it will look at the http status code & handle it or it wont. What I need is both. If there is an http status error (400/500) then I want the failure (even if there is no fault message in the body). If it's a 200 and there is a fault message I want it to fail as well. That setting makes it so only one of those cases can ever be handled in a single request. In my instance we are calling a 3rd party service (hosted externally by a 3rd party vendor) and so we have no control whatsoever what they are doing. In some instances there are 500s and others it's 200s with a fault message. We've told them they are deviating from the soap spec, but at the end of the day there is nothing we can do about it.

@gregturn
Copy link
Member Author

@gregturn gregturn commented Nov 17, 2015

Arjen Poutsma commented

Note that there are two properties: checkConnectionForError and checkConnectionForFault:

  • If checkConnectionForError is set to true (the default), WebServiceTemplate will check the HTTP status code for a non-200 status code and a throw a transport exception if it is.

  • If checkConnectionForError is set to false, the template not check the status code, and will never raise a transport exception.

  • If checkConnectionForFault is set to to true (the default), after checking for errors, the template will inspect the HTTP connection for a 500 error code. If it is, it might be a SOAP fault, so the message is parsed to see if it is indeed a SOAP fault.

  • If checkConnectionForFault is set to to false, the template will not check the status code for a 500, and always parses the message to see if it is a SOAP fault.

As you might understand, the main reason for inspecting the status code for a 500 is performance: it can be quite expensive to parse a complete SOAP message, which is required to see if it is a fault.

I do not know the service you are trying to acces, but I gather that setting checkConnectionForFault to false will work for the problem you describe. Did you try it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant