fixed #197 indexer crash on xml embeded schema with empty attribute n…

…ame; added regression to tests
manticoresoftware · Feb 8, 2018 · 3613714 · 3613714
1 parent a5563a4
commit 3613714
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 6 deletions.
diff --git a/src/gtests_text.cpp b/src/gtests_text.cpp
@@ -590,3 +590,47 @@ TEST ( Text, cvs_source )
 }
 
 
+//////////////////////////////////////////////////////////////////////////
+
+TEST ( Text, xml_source_attr_error )
+{
+	const char * sTest = 
+R"raw(<?xml version="1.0" encoding="utf-8"?>
+<sphinx:docset xmlns:sphinx="http://sphinxsearch.com/">
+<sphinx:schema>
+    <sphinx:attr name="" type="int" />
+    <sphinx:field name="f" />
+</sphinx:schema>
+<sphinx:document id="1">
+    <>9</>
+    <f>hey</f>
+</sphinx:document>
+</sphinx:docset>
+)raw";
+
+	const char * sRes = "source 'xml': (null) is not a valid attribute name (line=4, pos=4, docid=0)";
+
+	CSphString sTmpFile = "__libsphinxtestxml.xml";
+	// write xml file
+	FILE * fp = fopen ( sTmpFile.cstr(), "wb" );
+	fwrite ( sTest, 1, strlen ( sTest ), fp );
+	fclose ( fp );
+
+	// open csv pipe
+	fp = fopen ( sTmpFile.cstr(), "rb" );
+
+	CSphString sError;
+	// make config
+	CSphConfigSection tConf;
+
+	// setup source
+	CSphSource_Document * pSource = ( CSphSource_Document * ) sphCreateSourceXmlpipe2 ( &tConf, fp, "xml", 2*1024*1024, false, sError );
+	ASSERT_FALSE ( pSource->Connect ( sError ) );
+	ASSERT_STREQ ( sError.cstr(), sRes );
+
+	// clean up, fp will be closed automatically in CSphSource_BaseSV::Disconnect()
+	SafeDelete ( pSource );
+	unlink ( sTmpFile.cstr() );
+}
+
+
diff --git a/src/sphinx.cpp b/src/sphinx.cpp
@@ -27967,6 +27967,12 @@ struct CSphSchemaConfigurator
 				tCol.m_eSrc = SPH_ATTRSRC_FIELD;
 			}
 
+			if ( tCol.m_sName.IsEmpty() )
+			{
+				sError.SetSprintf ( "column number %d has no name", tCol.m_iIndex );
+				return false;
+			}
+
 			if ( CSphSchema::IsReserved ( tCol.m_sName.cstr() ) )
 			{
 				sError.SetSprintf ( "%s is not a valid attribute name", tCol.m_sName.cstr() );
@@ -28552,10 +28558,16 @@ bool CSphSource_XMLPipe2::ParseNextChunk ( int iBufferLen, CSphString & sError )
 		if ( m_dParsedDocuments.GetLength() )
 			uFailedID = m_dParsedDocuments.Last()->m_uDocID;
 
-		sError.SetSprintf ( "source '%s': XML parse error: %s (line=%d, pos=%d, docid=" DOCID_FMT ")",
+		if ( !m_sError.IsEmpty () )
-			m_tSchema.GetName(), sph_XML_ErrorString ( sph_XML_GetErrorCode ( m_pParser ) ),
+		{
-			(int)sph_XML_GetCurrentLineNumber ( m_pParser ), (int)sph_XML_GetCurrentColumnNumber ( m_pParser ),
+			sError = m_sError;
-			uFailedID );
+		} else
+		{
+			sError.SetSprintf ( "source '%s': XML parse error: %s (line=%d, pos=%d, docid=" DOCID_FMT ")",
+				m_tSchema.GetName(), sph_XML_ErrorString ( sph_XML_GetErrorCode ( m_pParser ) ),
+				(int)sph_XML_GetCurrentLineNumber ( m_pParser ), (int)sph_XML_GetCurrentColumnNumber ( m_pParser ),
+				uFailedID );
+		}
 		m_tDocInfo.m_uDocID = 1;
 		return false;
 	}
@@ -28814,7 +28826,7 @@ void CSphSource_XMLPipe2::StartElement ( const char * szName, const char ** pAtt
 
 		if ( bIsAttr )
 		{
-			if ( CSphSchema::IsReserved ( Info.m_sName.cstr() ) )
+			if ( Info.m_sName.IsEmpty() || CSphSchema::IsReserved ( Info.m_sName.cstr() ) )
 			{
 				Error ( "%s is not a valid attribute name", Info.m_sName.cstr() );
 				return;
@@ -28881,7 +28893,7 @@ void CSphSource_XMLPipe2::StartElement ( const char * szName, const char ** pAtt
 
 		if ( !bError )
 		{
-			if ( CSphSchema::IsReserved ( Info.m_sName.cstr() ) )
+			if ( Info.m_sName.IsEmpty() || CSphSchema::IsReserved ( Info.m_sName.cstr() ) )
 			{
 				Error ( "%s is not a valid attribute name", Info.m_sName.cstr() );
 				return;