Bug 14004 - GPO export fails when a GptTmpl.inf has a UTF BOM
Summary: GPO export fails when a GptTmpl.inf has a UTF BOM
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.10.5
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Samba QA Contact
QA Contact: Samba QA Contact
Depends on:
Reported: 2019-06-22 20:53 UTC by Kacper
Modified: 2020-03-10 09:47 UTC (History)
4 users (show)

See Also:

GptTmpl with BOM patch (1.25 KB, patch)
2019-07-18 02:52 UTC, Garming Sam
no flags Details
Backported patch for 4.11 (2.58 KB, patch)
2019-07-19 02:50 UTC, Garming Sam
garming: review? (abartlet)
dbagnall: review+
Backported patch for 4.10 (2.58 KB, patch)
2019-07-19 03:56 UTC, Garming Sam
dbagnall: review+
garming: review? (abartlet)
Patch for master changing XML encoding (1.45 KB, patch)
2019-10-15 00:41 UTC, Garming Sam
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kacper 2019-06-22 20:53:15 UTC
GPO export fails when a GptTmpl.inf has a UTF BOM. gp_inf.py line 322 fails if a GptTmpl.inf has a UTF BOM.
Comment 1 Garming Sam 2019-07-18 02:20:16 UTC
(In reply to Kacper from comment #0)

Do you have the backtrace? Looking at the code, it does in fact skip over the BOM, but I can't see how that might immediately cause an error.
Comment 2 Garming Sam 2019-07-18 02:52:12 UTC
Created attachment 15315 [details]
GptTmpl with BOM patch

Nevermind, I think I understand the issue now. Can you try with this patch to your code?
Comment 3 Garming Sam 2019-07-18 02:59:17 UTC
What version of Windows are you using to edit these settings by the way? It seems strange that I never encountered this until now.
Comment 4 Garming Sam 2019-07-18 03:27:17 UTC
It looks like this was a regression from a different change prior to the 4.10 release 16596842a62bec0a9d974c48d64000e3c079254e.
Comment 5 Garming Sam 2019-07-19 02:50:20 UTC
Created attachment 15316 [details]
Backported patch for 4.11
Comment 6 Garming Sam 2019-07-19 03:56:36 UTC
Created attachment 15317 [details]
Backported patch for 4.10
Comment 7 Garming Sam 2019-07-30 22:26:43 UTC
Assigning to Karolin for inclusion in the next 4.10 release and the next 4.11 RC.
Comment 8 Karolin Seeger 2019-08-06 08:14:29 UTC
Pushed to autobuild-v4-10-test
Comment 9 Karolin Seeger 2019-08-09 07:46:38 UTC
Pushed to v4-11-test.

Re-assigning to Garming as it seems to break the build on v4-10-test:

UNEXPECTED(failure): samba.tests.samba_tool.gpo.python2.samba.tests.samba_tool.gpo.GpoCmdTestCase.test_backup_restore_backup_compare_XML(ad_dc_ntvfs:local)
REASON: Exception: Exception: Traceback (most recent call last):
  File "/memdisk/kseeger/a410/b392271/samba-py2/bin/python/samba/tests/samba_tool/gpo.py", line 283, in test_backup_restore_backup_compare_XML
    self.assertCmdSuccess(result, out, err, "Ensuring gpo fetched successfully")
  File "/memdisk/kseeger/a410/b392271/samba-py2/bin/python/samba/tests/samba_tool/base.py", line 109, in assertCmdSuccess
    exit, out, err, msg))
AssertionError: exit[-1] stdout[] stderr[ERROR(<class 'xml.parsers.expat.ExpatError'>): Error copying GPO from DC - encoding specified in XML declaration is incorrect: line 1, column 30
  File "/memdisk/kseeger/a410/b392271/samba-py2/bin/python/samba/netcmd/gpo.py", line 1050, in run
    backup_directory_remote_to_local(conn, sharepath, gpodir)
  File "/memdisk/kseeger/a410/b392271/samba-py2/bin/python/samba/netcmd/gpo.py", line 295, in backup_directory_remote_to_local
    parser.write_xml(l_name + '.xml')
  File "/memdisk/kseeger/a410/b392271/samba-py2/bin/python/samba/gp_parse/gp_inf.py", line 353, in write_xml
    self.write_pretty_xml(root, f)
  File "/memdisk/kseeger/a410/b392271/samba-py2/bin/python/samba/gp_parse/__init__.py", line 81, in write_pretty_xml
    minidom_parsed = minidom.parseString(temporary_bytes.getvalue())
  File "/usr/lib/python2.7/xml/dom/minidom.py", line 1928, in parseString
    return expatbuilder.parseString(string)
  File "/usr/lib/python2.7/xml/dom/expatbuilder.py", line 940, in parseString
    return builder.parseString(string)
  File "/usr/lib/python2.7/xml/dom/expatbuilder.py", line 223, in parseString
    parser.Parse(string, True)
]: Ensuring gpo fetched successfully

FAILED (1 failures, 0 errors and 0 unexpected successes in 0 testsuites)
Comment 10 Karolin Seeger 2019-08-09 07:48:49 UTC
(In reply to Karolin Seeger from comment #9)
It seems to break the test, not the build ;-)
Comment 11 Garming Sam 2019-08-11 22:41:07 UTC
(In reply to Karolin Seeger from comment #10)

Sorry Karolin! I think there might be another bug... It passes with python3 on my machine. I'll take a look at it some more.
Comment 12 Kacper 2019-10-07 12:26:30 UTC
I've tested the patch that landed in 4.11 however there is a vital part missing. While the patch saves the migrated GptTmpl.inf.xml as UTF-16LE, the part in gpo.py (with open(l_name, 'r') as ltemp:) that reads the resulting xml file only handles utf-8.
Comment 13 Garming Sam 2019-10-07 20:20:25 UTC
(In reply to Kacper from comment #12)

The xml file is always supposed to be utf-8. It's only when the file is restored that it is restored as utf-16le. If you're still running into issues, can you show me a backtrace (and possibly the GptTmpl that is causing issue)?
Comment 14 Garming Sam 2019-10-15 00:41:24 UTC
Created attachment 15538 [details]
Patch for master changing XML encoding

Haven't got a test, but this patch fixes the encoding of the XML which was botched with the change.
Comment 15 Andrew Bartlett 2020-03-10 09:47:32 UTC
While the regression for sites that had used the backup before these patches is not addressed, the main issue here is fixed in all supported Samba versions (4.10 is security releases only now).