Bug 1091461

Description of problem:

When the configchannel.createOrUpdatePath API creates a new revision for an existing text or binary file, the contents are corrupted, ending up as a string of null characters.

Version-Release number of selected component (if applicable):

spacewalk-backend-server-2.2.20-1

How reproducible:

100%

Steps to Reproduce:

1.) Create a new text or binary config file in the web UI or via the configfile.createOrUpdatePath API.

2.) Upload a new revision to this file via the configfile.createOrUpdatePath API.

3.) Verify in the web UI or via the configfile.getFileRevision API that the new revision does not match what was uploaded.

Actual results:

Content of the new config file revision uploaded via configfile.createOrUpdatePath API is corrupt.

Expected results:

Successful config file revision update.

Additional info:

Reproducer script, which uploads a revision of the sample small file /root/file.gz from the local system to a config channel called 'test-config-channel', as a binary file using base64 encoding:

----
#!/usr/bin/python

import xmlrpclib
import base64

SATELLITE_URL = "http://satellite.example.com/rpc/api"
SATELLITE_LOGIN = "admin"
SATELLITE_PASSWORD = "XXXX"

client = xmlrpclib.Server(SATELLITE_URL, verbose=0)
key = client.auth.login(SATELLITE_LOGIN, SATELLITE_PASSWORD)

configChannelLabel = 'test-config-channel'
configFilePath = '/root/file.gz'

fileInfoDict = client.configchannel.lookupFileInfo(key, configChannelLabel, [configFilePath])

fileDict = client.configchannel.getFileRevision(key, configChannelLabel, configFilePath, fileInfoDict[0]['revision'])

print "Current config file revision: ", fileDict['contents']

configDict={}

with open(configFilePath, "r") as fd:
    data = base64.b64encode(fd.read())

configDict["contents"] = data
configDict["contents_enc64"] = True
configDict["owner"] = "root"
configDict["group"] = "root"
configDict["permissions"] = "644"
configDict["selinux_ctx"] = ""
configDict["macro-start-delimiter"] = ""
configDict["macro-end-delimiter"] = ""
configDict["binary"] = True

print "data = ", data
print "Updating config file"
returnDict = client.configchannel.createOrUpdatePath(key, configChannelLabel, configFilePath, False, configDict)

print "returnDict = ", returnDict

fileDict = client.configchannel.getFileRevision(key, configChannelLabel, configFilePath, returnDict['revision'])

print "New config file revision: ", fileDict['contents']

client.auth.logout(key)
----

After modifying /root/file.gz and running the script, the output is:

****
# ./config-files.py 
Current config file revision:  H4sICHscWVMAA2ZpbGUAK0ktLuEqQSW4AJXkCOUVAAAA
data =  H4sICCiHWlMAA2ZpbGUAK0ktLjHkAgAbN1l8BgAAAA==
Updating config file
returnDict =  {'binary': True, 'group': 'root', 'channel': 'tpapaioa-config', 'creation': <DateTime '20140425T12:02:57' at 7f4fa22a2d40>, 'modified': <DateTime '20140425T12:02:57' at 7f4fa22a2d88>, 'contents_enc64': True, 'permissions_mode': '644', 'contents': 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==', 'owner': 'root', 'path': '/root/file.gz', 'permissions': 644, 'type': 'file', 'md5': '3861facee9efc127e340387f1936b8fb', 'revision': 32}
****

Note that the contents returned by the createOrUpdatePath API is a string of A's (nulls), not the actual file contents shown as "data" in the script output.

The server-side code causing the problem is below:

****
./code/src/com/redhat/rhn/frontend/xmlrpc/configchannel/XmlRpcConfigChannelHelper.java:

    public ConfigRevision createOrUpdatePath(User loggedInUser,
                                         ConfigChannel channel,
                                         String path,
                                         ConfigFileType type,
                                         Map<String, Object> data) {
[...]
        if (ConfigFileType.file().equals(type)) {
            try {
                if (BooleanUtils.isTrue((Boolean) data.get(
                                ConfigRevisionSerializer.BINARY))) {
                    byte[] content = Base64.decodeBase64(
                            ((String)data.get(ConfigRevisionSerializer.CONTENTS))
                            .getBytes("UTF-8"));

                    if (content != null) {
                        form = new BinaryFileData(new ByteArrayInputStream(content),
                                                                        content.length);
                    }
                    else {
                        form = new BinaryFileData(new ByteArrayInputStream(new byte[0]), 0);
                    }
                }
****
The content stream for the BinaryFileData object "form" is read twice, once by matchesRevision, and once by createNewContentFromStream:

****
com/redhat/rhn/manager/configuration/ConfigFileBuilder.java:

    private ConfigRevision makeNewRevision(User user, ConfigFileData form,
            ConfigFile cf, boolean onCreate) {
[...]
            if ((prevRevision != null) &&
                    form.matchesRevision(prevRevision)) {
               return prevRevision;
            }
[...]
        revision.setConfigContent(
                ConfigurationFactory.createNewContentFromStream(
                        form.getContentStream(), form.getContentSize(),
                        form.isBinary(), delimStart, delimEnd));
****

Because it's a stream, the first read advances the offset, so that the 2nd read just gets null output. This results in null output stored in the new revision. I've verified that a quick fix is to use mark() and reset() in matchesRevision, so that the offset is unaffected after the comparison of the new content to the last revision. Proposed patch is available at:

https://github.com/spacewalkproject/spacewalk/pull/52