Skip to content

Commit ddb8037

Browse files
committed
Fix bug with symlink creation
Previously, this was executing a command using a shell, but without properly escaping the input. This instead passes a list in to Popen, to ensure escape is working properly.
1 parent ce2a853 commit ddb8037

File tree

3 files changed

+58
-1
lines changed

3 files changed

+58
-1
lines changed

readthedocs/core/symlink.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,13 @@ def symlink_subprojects(self):
198198
symlink_dir = os.sep.join(symlink.split(os.path.sep)[:-1])
199199
if not os.path.lexists(symlink_dir):
200200
safe_makedirs(symlink_dir)
201-
run('ln -nsf %s %s' % (docs_dir, symlink))
201+
# TODO this should use os.symlink, not a call to shell. For now,
202+
# this passes command as a list to be explicit about escaping
203+
# spaces.
204+
status, _, stderr = run(['ln', '-nsf', docs_dir, symlink])
205+
if status > 0:
206+
log.error('Could not symlink path: status=%d error=%s',
207+
status, stderr)
202208

203209
# Remove old symlinks
204210
if os.path.exists(self.subproject_root):
@@ -251,8 +257,10 @@ def symlink_single_version(self):
251257
# Clean up symlinks
252258
symlink = self.project_root
253259
if os.path.islink(symlink):
260+
self._log('Unlinking single version: {0}'.format(symlink))
254261
os.unlink(symlink)
255262
if os.path.exists(symlink):
263+
self._log('Unlinking single version: {0}'.format(symlink))
256264
shutil.rmtree(symlink)
257265

258266
# Create symlink

readthedocs/projects/utils.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ def run(*commands, **kwargs):
7272
if shell:
7373
log.info("Running commands in a shell")
7474
run_command = command
75+
elif isinstance(command, list):
76+
run_command = command
7577
else:
7678
run_command = command.split()
7779
log.info("Running: '%s' [%s]", command, cwd)

readthedocs/rtd_tests/tests/test_project_symlinks.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,53 @@ def test_subproject_alias(self):
322322
filesystem['private_web_root'] = public_root
323323
self.assertFilesystem(filesystem)
324324

325+
def test_subproject_alias_with_spaces(self):
326+
"""Symlink pass adds symlink for subproject alias"""
327+
self.project.add_subproject(self.subproject, alias='Sweet Alias')
328+
self.symlink.symlink_subprojects()
329+
filesystem = {
330+
'private_cname_project': {},
331+
'private_cname_root': {},
332+
'private_web_root': {
333+
'kong': {'en': {}},
334+
'sub': {'en': {}},
335+
},
336+
'public_cname_project': {},
337+
'public_cname_root': {},
338+
'public_web_root': {
339+
'kong': {
340+
'en': {'latest': {
341+
'type': 'link',
342+
'target': 'user_builds/kong/rtd-builds/latest',
343+
}},
344+
'projects': {
345+
'sub': {
346+
'type': 'link',
347+
'target': 'public_web_root/sub',
348+
},
349+
'Sweet Alias': {
350+
'type': 'link',
351+
'target': 'public_web_root/sub',
352+
},
353+
}
354+
},
355+
'sub': {
356+
'en': {'latest': {
357+
'type': 'link',
358+
'target': 'user_builds/sub/rtd-builds/latest',
359+
}}
360+
}
361+
}
362+
}
363+
if self.privacy == 'private':
364+
public_root = filesystem['public_web_root'].copy()
365+
private_root = filesystem['private_web_root'].copy()
366+
public_root['kong']['projects']['sub']['target'] = 'private_web_root/sub'
367+
public_root['kong']['projects']['Sweet Alias']['target'] = 'private_web_root/sub'
368+
filesystem['public_web_root'] = private_root
369+
filesystem['private_web_root'] = public_root
370+
self.assertFilesystem(filesystem)
371+
325372
def test_remove_subprojects(self):
326373
"""Nonexistant subprojects are unlinked"""
327374
self.project.add_subproject(self.subproject)

0 commit comments

Comments
 (0)