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

Allowing tubes buffer size to be variable defaulted #803

Closed
wants to merge 4 commits into from
Closed

Allowing tubes buffer size to be variable defaulted #803

wants to merge 4 commits into from

Conversation

bannsec
Copy link
Contributor

@bannsec bannsec commented Dec 1, 2016

In working with large return values on pwntools, i noticed there's no good way to get them to return properly. For instance, I was attempting to do a "recvuntil" on data that was larger than 4096. Even going in and changing the default values to higher didn't have the intended change since the fillbuffer method had statically mapped in 4096.

The proposed changes simply allow a variable when creating process/remote objects that allow the user to define the buffer size. 4096 is fine for most, so I have left that as the default. However, it is now possible to define your own size with "bufSize=9999" or whatever. In my example problematic situation, this change solved the problem.

@zachriggle
Copy link
Member

zachriggle commented Dec 1, 2016

First, thanks for the contribution.

As it stands, this needs to be rebased on top of the current dev branch before we can do anything with it.

This property is also a bit misleading. The name bufSize is confusing, since each tube object also has a Buffer object, but it has nothing to do with that.

We generally use snake_case instead of camelCase, as well.

It might make the most sense to put this property on the Buffer class directly as buffer_fill_size. Instead of adding the attribute to each sub-class's constructor, ust use *args, **kwargs to allow passing arbitrary named parameters up the constructor stack. Each tube already has a Buffer object, so that gets handled magically.

It's also likely worth adding a helper routine Buffer.get_fill_size(self, size) which handles the self.bufSize if foo is None else foo logic in a compartmented format.

Finally, you missed several tube classes, which will need to have their constructors updated.

  • listen
  • serialtube
  • ssh_channel
  • ssh_listener

@zachriggle zachriggle added this to the Someday milestone Dec 1, 2016
@zachriggle zachriggle self-assigned this Dec 1, 2016
@bannsec
Copy link
Contributor Author

bannsec commented Dec 3, 2016

Gotcha. Not sure how to do all this in one pull request. Think I'll just submit a second one to hopefully make the merge cleaner.

@zachriggle
Copy link
Member

zachriggle commented Dec 3, 2016 via email

@zachriggle
Copy link
Member

Superceded by #806

@zachriggle zachriggle closed this Dec 5, 2016
@zachriggle zachriggle removed this from the Someday milestone Dec 5, 2016
Kyle-Kyle pushed a commit to Kyle-Kyle/pwntools that referenced this pull request Apr 25, 2021
Buffer all repeating lines and check the minimum value when to start
marking them with skip lines. In case the minimum value is not hit, just
unroll the buffer.

To stop skipping any lines, there is the existing bool config
telescope-skip-repeat-val so we avoid adding special values to minimum
like -1 and keep the setting separated.

Fixes Gallopsled#803
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants