Last Comment Bug 337179 - Schema with forward type definitions causing load errors
: Schema with forward type definitions causing load errors
Status: RESOLVED FIXED
: fixed1.8.1.1
Product: Core
Classification: Components
Component: Web Services (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Doron Rosenberg (IBM)
: Stephen Pride
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-05-08 13:43 PDT by Steve Speicher
Modified: 2007-01-11 16:16 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test case (593 bytes, application/xml)
2006-05-08 13:44 PDT, Steve Speicher
no flags Details
driver XForm with link to schema with problem (856 bytes, application/xhtml+xml)
2006-05-08 13:45 PDT, Steve Speicher
no flags Details
schema that works (599 bytes, application/xml)
2006-05-08 13:46 PDT, Steve Speicher
no flags Details
le patch (4.68 KB, patch)
2006-05-09 14:48 PDT, Doron Rosenberg (IBM)
aaronr: review+
dveditz: approval1.8.0.8-
mtschrep: approval1.8.1-
jaymoz: approval1.8.1.1+
Details | Diff | Splinter Review

Description Steve Speicher 2006-05-08 13:43:28 PDT
User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1) Opera 7.55  [en] (IBM EVV/3.8/EAK01AGF/LE)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060508 Minefield/3.0a1

Schema definition like this (complexType forward referencing another complexType) is the cause of the problem:
  <complexType name="fooType">
    <simpleContent>
      <restriction base="tns:textType">
        <maxLength value="2" />
      </restriction>
    </simpleContent>
  </complexType>

  <complexType name="textType">
    <simpleContent>
      <extension base="string">
      </extension>
    </simpleContent>
  </complexType>


Reproducible: Always
Comment 1 Steve Speicher 2006-05-08 13:44:10 PDT
Created attachment 221350 [details]
test case
Comment 2 Steve Speicher 2006-05-08 13:45:58 PDT
Created attachment 221351 [details]
driver XForm with link to schema with problem
Comment 3 Steve Speicher 2006-05-08 13:46:27 PDT
Created attachment 221352 [details]
schema that works
Comment 4 Doron Rosenberg (IBM) 2006-05-09 14:48:58 PDT
Created attachment 221501 [details] [diff] [review]
le patch

this should fix it.  Steve, can you test with your larger forms?

Parts of the code are not schema placeholder (if a type is defined after a user is found) compliant, this makes them behave again.
Comment 5 Steve Speicher 2006-05-10 10:17:02 PDT
(In reply to comment #4)

> Steve, can you test with your larger forms?
> 

Yes and along with patch for bug 337179, works nicely.
Comment 6 Steve Speicher 2006-05-12 12:34:35 PDT
(In reply to comment #5)
> (In reply to comment #4)
> 
> > Steve, can you test with your larger forms?
> > 
> 
> Yes and along with patch for bug 337179, works nicely.
> 

I meant with bug 335366 (though not strictly dependent on it)
Comment 7 aaronr 2006-05-24 15:08:10 PDT
Comment on attachment 221501 [details] [diff] [review]
le patch


>Index: extensions/webservices/schema/src/nsSchemaLoader.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/webservices/schema/src/nsSchemaLoader.cpp,v
>retrieving revision 1.43.2.3
>diff -u -p -1 -0 -r1.43.2.3 nsSchemaLoader.cpp
>--- extensions/webservices/schema/src/nsSchemaLoader.cpp	15 Mar 2006 17:35:52 -0000	1.43.2.3
>+++ extensions/webservices/schema/src/nsSchemaLoader.cpp	9 May 2006 21:45:24 -0000
>@@ -1840,43 +1840,51 @@ nsSchemaLoader::ProcessSimpleContentRest
>  
>   restrictionInst = new nsSchemaRestrictionType(aSchema, EmptyString());
>   if (!restrictionInst) {
>     return NS_ERROR_OUT_OF_MEMORY;
>   }
>   simpleBase = restrictionInst;
>   
>   // The base type must actually be a complex type (which itself must
>   // have a simple base type.
>   nsCOMPtr<nsISchemaComplexType> complexBase = do_QueryInterface(aBaseType);
>-  if (!complexBase) {
>-    nsAutoString baseStr;
>-    rv = aBaseType->GetName(baseStr);
>-    NS_ENSURE_SUCCESS(rv, rv);
>-
>-    nsAutoString errorMsg;
>-    errorMsg.AppendLiteral("Failure processing schema, base type \"");
>-    errorMsg.Append(baseStr);
>-    errorMsg.AppendLiteral("\" of restriction must be a complex type ");
>-    errorMsg.AppendLiteral("which itself must be based on a simple type");
>+    if (!complexBase) {
>+    // if base type is a place holder, this is ok
>+    PRUint16 schemaType;
>+    aBaseType->GetSchemaType(&schemaType);

nit: indentation

>+
>+    if (schemaType == nsISchemaType::SCHEMA_TYPE_PLACEHOLDER) {
>+      simpleBase = do_QueryInterface(aBaseType);

you don't check the return type from GetSchemaType and you don't assign an initial value to schemaType.  You like walking the razor's edge!  fix please.

>+    } else {
>+      nsAutoString baseStr;
>+      rv = aBaseType->GetName(baseStr);
>+      NS_ENSURE_SUCCESS(rv, rv);
> 
>-    NS_SCHEMALOADER_FIRE_ERROR(NS_ERROR_SCHEMA_INVALID_TYPE_USAGE, errorMsg);
>+      nsAutoString errorMsg;
>+      errorMsg.AppendLiteral("Failure processing schema, base type \"");
>+      errorMsg.Append(baseStr);
>+      errorMsg.AppendLiteral("\" of restriction must be a complex type ");
>+      errorMsg.AppendLiteral("which itself must be based on a simple type");
> 
>-    return NS_ERROR_SCHEMA_INVALID_TYPE_USAGE;
>-  }
>+      NS_SCHEMALOADER_FIRE_ERROR(NS_ERROR_SCHEMA_INVALID_TYPE_USAGE, errorMsg);
> 
>-  nsCOMPtr<nsISchemaSimpleType> parentSimpleBase;
>-  complexBase->GetSimpleBaseType(getter_AddRefs(parentSimpleBase));
>+      return NS_ERROR_SCHEMA_INVALID_TYPE_USAGE;
>+    }
>+  } else {
>+    nsCOMPtr<nsISchemaSimpleType> parentSimpleBase;
>+    complexBase->GetSimpleBaseType(getter_AddRefs(parentSimpleBase));
>   
>-  if (parentSimpleBase) {
>-    rv = restrictionInst->SetBaseType(parentSimpleBase);
>-    if (NS_FAILED(rv)) {
>-      return rv;
>+    if (parentSimpleBase) {
>+      rv = restrictionInst->SetBaseType(parentSimpleBase);
>+      if (NS_FAILED(rv)) {
>+        return rv;
>+      }

nit: NS_ENSURE_SUCCESS(rv, rv)

nothing major that I noticed.  r=me
Comment 8 Allan Beaufour 2006-05-25 08:09:04 PDT
Comment on attachment 221501 [details] [diff] [review]
le patch

I do not see something obviously wrong, except for the remarks that Aaron had. But I do not really feel qualified for r+ in that part of the code....
Comment 9 aaronr 2006-09-24 23:34:14 PDT
Comment on attachment 221501 [details] [diff] [review]
le patch

great to have for the next 1.8.0.x release
Comment 10 Mike Schroepfer 2006-09-25 19:10:38 PDT
Comment on attachment 221501 [details] [diff] [review]
le patch

Doesn't meet criteria for RC2.   i.e. "Show Stopper Bugs" - major regression, crash, security issue.  We can pickup in a follow-on release.
Comment 11 Daniel Veditz [:dveditz] 2006-09-26 15:21:05 PDT
Comment on attachment 221501 [details] [diff] [review]
le patch

1.8.0.8 is to keep 1.8.0.x security fixes in-line with the FF2 release, we're not taking things they aren't.
Comment 12 aaronr 2006-10-31 13:58:02 PST
Comment on attachment 221501 [details] [diff] [review]
le patch

Now that FF 2 is out, can we get this in?
Comment 13 Daniel Veditz [:dveditz] 2006-11-29 15:24:44 PST
Comment on attachment 221501 [details] [diff] [review]
le patch

Doesn't appear to have a super review, This is going to have to wait.
Comment 14 Doron Rosenberg (IBM) 2006-11-30 06:58:19 PST
Patch was written by module owner and was simple enough to not need a sr.
Comment 15 Reed Loden [:reed] (use needinfo?) 2006-11-30 08:32:03 PST
(In reply to comment #14)
> Patch was written by module owner and was simple enough to not need a sr.

Rerequest approval1.8.1.1, or your message will never be seen by the drivers.
Comment 16 Jay Patel [:jay] 2006-11-30 11:33:13 PST
Comment on attachment 221501 [details] [diff] [review]
le patch

Given Doron's last comment, approved for 1.8 branch, a= jay for drivers.
Comment 17 aaronr 2007-01-11 16:16:13 PST
doesn't sound eligible for 1.8.0 branch, so removing xf-to-branch

Note You need to log in before you can comment on or make changes to this bug.