Closed Bug 202478 Opened 21 years ago Closed 21 years ago

Potential stack over flow if a wsdl file is imported recursively

Categories

(Core Graveyard :: Web Services, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: harishd, Assigned: harishd)

References

Details

Attachments

(2 files)

If a wsdl file, containing an import element, imports itself then a
synchronously loaded wsdl ( via sync. proxy load ) can result in stack overflow
Attached patch patch v1.0Splinter Review
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.
Attachment #120920 - Flags: superreview?(jst)
Attachment #120920 - Flags: review?(nisheeth)
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?
>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.
Status: NEW → ASSIGNED
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 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
Attachment #120920 - Flags: review?(nisheeth) → review+
Comment on attachment 120920 [details] [diff] [review]
patch v1.0

Agreed. A specific error would be good.

sr=jst with that change.
Attachment #120920 - Flags: superreview?(jst) → superreview+
Attached patch patch v1.0.1Splinter Review
Contains the suggested change.
Comment on attachment 121003 [details] [diff] [review]
patch v1.0.1

Carrying forward r/sr
Attachment #121003 - Flags: superreview+
Attachment #121003 - Flags: review+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Blocks: 190182
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: