Schema with forward type definitions causing load errors

RESOLVED FIXED

Status

()

Core
Web Services
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: Steve Speicher, Assigned: Doron Rosenberg (IBM))

Tracking

({fixed1.8.1.1})

Trunk
fixed1.8.1.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

11 years ago
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
(Reporter)

Comment 1

11 years ago
Created attachment 221350 [details]
test case
(Reporter)

Comment 2

11 years ago
Created attachment 221351 [details]
driver XForm with link to schema with problem
(Reporter)

Comment 3

11 years ago
Created attachment 221352 [details]
schema that works
(Assignee)

Comment 4

11 years ago
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.
(Reporter)

Comment 5

11 years ago
(In reply to comment #4)

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

Yes and along with patch for bug 337179, works nicely.
(Reporter)

Comment 6

11 years ago
(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)

Updated

11 years ago
Assignee: aaronr → doronr
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

11 years ago
Attachment #221501 - Flags: review?(aaronr)

Comment 7

11 years ago
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+
(Assignee)

Updated

11 years ago
Attachment #221501 - Flags: review?(allan)

Comment 8

11 years ago
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)
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch

Updated

11 years ago
Component: XForms → Web Services
OS: Windows XP → All
Hardware: PC → All

Comment 9

11 years ago
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?

Updated

11 years ago
Attachment #221501 - Flags: approval1.8.1?
Summary: Schema with forward type defintions causing load errors → Schema with forward type definitions causing load errors

Comment 10

11 years ago
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 12

11 years ago
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-
(Assignee)

Comment 14

11 years ago
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 16

11 years ago
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+
(Assignee)

Updated

11 years ago
Keywords: fixed1.8.1.1

Comment 17

10 years ago
doesn't sound eligible for 1.8.0 branch, so removing xf-to-branch
Whiteboard: xf-to-branch
You need to log in before you can comment on or make changes to this bug.