Closed Bug 337179 Opened 14 years ago Closed 14 years ago

Schema with forward type definitions causing load errors

Categories

(Core Graveyard :: Web Services, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: sspeiche, Assigned: doronr)

Details

(Keywords: fixed1.8.1.1)

Attachments

(4 files)

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
Attached file test case
Attached file schema that works
Attached patch le patchSplinter Review
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.
(In reply to comment #4)

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

Yes and along with patch for bug 337179, works nicely.
(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)
Assignee: aaronr → doronr
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #221501 - Flags: review?(aaronr)
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
Attachment #221501 - Flags: review?(aaronr) → review+
Attachment #221501 - Flags: review?(allan)
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....
Attachment #221501 - Flags: review?(allan)
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
Component: XForms → Web Services
OS: Windows XP → All
Hardware: PC → All
Comment on attachment 221501 [details] [diff] [review]
le patch

great to have for the next 1.8.0.x release
Attachment #221501 - Flags: approval1.8.0.8?
Attachment #221501 - Flags: approval1.8.1?
Summary: Schema with forward type defintions causing load errors → Schema with forward type definitions causing load errors
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.
Attachment #221501 - Flags: approval1.8.1? → approval1.8.1-
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.
Attachment #221501 - Flags: approval1.8.0.8? → approval1.8.0.8-
Comment on attachment 221501 [details] [diff] [review]
le patch

Now that FF 2 is out, can we get this in?
Attachment #221501 - Flags: approval1.8.1.1?
Comment on attachment 221501 [details] [diff] [review]
le patch

Doesn't appear to have a super review, This is going to have to wait.
Attachment #221501 - Flags: approval1.8.1.2?
Attachment #221501 - Flags: approval1.8.1.1?
Attachment #221501 - Flags: approval1.8.1.1-
Patch was written by module owner and was simple enough to not need a sr.
(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 on attachment 221501 [details] [diff] [review]
le patch

Given Doron's last comment, approved for 1.8 branch, a= jay for drivers.
Attachment #221501 - Flags: approval1.8.1.2?
Attachment #221501 - Flags: approval1.8.1.1-
Attachment #221501 - Flags: approval1.8.1.1+
Keywords: fixed1.8.1.1
doesn't sound eligible for 1.8.0 branch, so removing xf-to-branch
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.