Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

picked cherries for 1.0-maint #2284

Merged
merged 13 commits into from Mar 11, 2017
Merged

picked cherries for 1.0-maint #2284

merged 13 commits into from Mar 11, 2017

Conversation

enkore
Copy link
Contributor

@enkore enkore commented Mar 10, 2017

enkore and others added 10 commits March 10, 2017 18:30
# Conflicts:
#	docs/development.rst
we do not trust the remote, so we are careful unpacking its responses.

the remote could return manipulated msgpack data that announces e.g.
a huge array or map or string. the local would then need to allocate huge
amounts of RAM in expectation of that data (no matter whether really
that much is coming or not).

by using limits in the Unpacker, a ValueError will be raised if unexpected
amounts of data shall get unpacked. memory DoS will be avoided.

# Conflicts:
#	borg/archiver.py
#	src/borg/archive.py
#	src/borg/remote.py
#	src/borg/repository.py
also: add some missing assertion messages

severity:

- no issue on little-endian platforms (== most, including x86/x64)
- harmless even on big-endian as long as refcount is below 0xfffbffff,
  which is very likely always the case in practice anyway.
* trigger bug in --verify-data, see borgbackup#2221

* raise decompression errors as DecompressionError, fixes borgbackup#2221

this is a subclass of IntegrityError, so borg check --verify-data works correctly if
the decompressor stumbles over corrupted data before the plaintext gets verified
(in a unencrypted repository, otherwise the MAC check would fail first).

* fixup: fix exception docstring, add placeholder, change wording
@enkore enkore changed the title picked cherries picked cherries for 1.0-maint Mar 10, 2017
@enkore enkore modified the milestones: 1.0.11, 1.0.11rc1 Mar 10, 2017
@codecov-io
Copy link

codecov-io commented Mar 10, 2017

Codecov Report

Merging #2284 into 1.0-maint will decrease coverage by 0.15%.
The diff coverage is 58.82%.

@@             Coverage Diff              @@
##           1.0-maint   #2284      +/-   ##
============================================
- Coverage      81.96%   81.8%   -0.16%     
============================================
  Files             15      15              
  Lines           5172    5210      +38     
  Branches         880     887       +7     
============================================
+ Hits            4239    4262      +23     
- Misses           676     687      +11     
- Partials         257     261       +4
Impacted Files Coverage Δ
borg/repository.py 87.87% <100%> (+0.01%)
borg/archive.py 80.34% <37.03%> (-1.17%)
borg/archiver.py 81.28% <60.56%> (+0.52%)
borg/remote.py 78.15% <80%> (-0.08%)
borg/helpers.py 87.8% <80%> (-0.13%)
borg/locking.py 89.58% <0%> (-1.05%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5212175...be1227f. Read the comment docs.

borg/archive.py Outdated
if e.errno == errno.E2BIG:
logger.warning('%s: Value or key of extended attribute %s is too big for this filesystem' %
(path, k.decode()))
elif e.errno not in (errno.ENOTSUP, errno.EACCES):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a bit strange now: warns if we could not extract xattrs (because they were too big), but says nothing if we could not extract at all (because not supported or not permitted). this was fixed in master.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to cherrypick #2258 for this.

if hashindex.API_VERSION != '1.0_01':
raise ExtensionModuleError
if chunker.API_VERSION != '1.0_01':
raise ExtensionModuleError
if compress.API_VERSION != '1.0_01':
raise ExtensionModuleError
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh? was this missing still in 1.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems so

elif kind == 'client':
args.update(dict(max_array_len=LIST_SCAN_LIMIT, # result list from repo.list() / .scan()
max_map_len=100, # misc. result dicts
))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these might be not the tightest limits as 1.0 does not use the dict-based rpc protocol, but should work I guess.

@@ -245,7 +245,7 @@ def cmd(self, *args, **kw):
return output

def create_src_archive(self, name):
self.cmd('create', self.repository_location + '::' + name, src_dir)
self.cmd('create', '--compression=lz4', self.repository_location + '::' + name, src_dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, do we want that in 1.0?

Copy link
Contributor Author

@enkore enkore Mar 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't make much of a difference, but might be good to have when backporting new tests that expect it.

@enkore
Copy link
Contributor Author

enkore commented Mar 10, 2017

Okay, so I'm going to pick up #2199 and #2258 as well

…borgbackup#2258)

* Set warning exit code when xattr is too big

* Warnings for more extended attributes errors (ENOTSUP, EACCES)

* Add tests for all xattr warnings

(cherry picked from commit 63b5cbf)
@enkore
Copy link
Contributor Author

enkore commented Mar 10, 2017

And #2184 (but neither #2204 nor #2255, correct)?

@ThomasWaldmann
Copy link
Member

@enkore yup. let's test the fixed async processing in master / 1.1b first for a while, then we can think whether we want it in 1.0 also.

@enkore
Copy link
Contributor Author

enkore commented Mar 10, 2017

Done

# note: might crash in compact() after committing the repo
repository.commit()
logger.info('Done. Run "borg check --repair" to clean up the mess.')
return self.exit_code
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the error reporting here to match the the singular-behaviour that borg delete normally has in 1.0.x (RC=2 if archive not found, no plural messages etc.)

Copy link
Member

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

for easier review, let's merge this and start a new PR for further stuff.

@enkore enkore merged commit 13455ab into borgbackup:1.0-maint Mar 11, 2017
@enkore enkore deleted the cp1 branch March 11, 2017 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants