202478 - Potential stack over flow if a wsdl file is imported recursively

Status: RESOLVED FIXED

(Core Graveyard :: Web Services, defect)

Description

[harishd]

If a wsdl file, containing an import element, imports itself then a
synchronously loaded wsdl ( via sync. proxy load ) can result in stack overflow

Comment 1

[harishd]

Attachment: patch v1.0

Comment 2

[harishd]

Comment on attachment 120920 [details] [diff] [review]
patch v1.0

Basically, we now maintain a URI list ( list of imported URIs ) and go thro'
the list before loading the URI in question.

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 120920 [details] [diff] [review]
patch v1.0

Don't you need to remove URI's from this list once they're loaded too? If not,
we wouldn't support a WSDL file importing the same file twice
(non-recursively), or is that not supposed to work either?

Comment 4

[harishd]

>Don't you need to remove URI's from this list once they're loaded too?
The life time of the WSDL loader is the time taken to load the WSDL file loaded
via proxy. That is, the list should not be a problem because the the same WSDL
loader will not be used to load a different WSDL file. But to be on the safe
side, jst and I agree that the list should be cleaned up before a new load. Will
attach a patch soon.

Comment 5

[harishd]

Poked around a little bit and found out that a new load request is created for
every new load and since the import list is a part of the load request cleaning
up the list should not be an issue. In other words, patch v1.0 should be good
enough :)

Comment 6

[Nisheeth Ranjan]

Comment on attachment 120920 [details] [diff] [review]
patch v1.0

I think NS_ERROR_WSDL_LOADING_ERROR is too generic a return value for error for
this specific case where we know that the IMPORT element is importing
recursively.  Lets create a new error constant, NS_ERROR_WSDL_RECURSIVE_IMPORT,
or something like that which captures this condition so that we can display
text based on this error whenever we decide to expose more informative error
messages to the user.

Apart from this suggestion, r=nisheeth

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 120920 [details] [diff] [review]
patch v1.0

Agreed. A specific error would be good.

sr=jst with that change.

Comment 8

[harishd]

Attachment: patch v1.0.1

Contains the suggested change.

Comment 9

[harishd]

Comment on attachment 121003 [details] [diff] [review]
patch v1.0.1

Carrying forward r/sr

Comment 10

[harishd]

Fixed.