Skip to content

Fix SSH shell arguments and environment handling#105

Open
binaryfire wants to merge 1 commit into
php:masterfrom
binaryfire:fix/ssh2-shell-environment-handling
Open

Fix SSH shell arguments and environment handling#105
binaryfire wants to merge 1 commit into
php:masterfrom
binaryfire:fix/ssh2-shell-environment-handling

Conversation

@binaryfire

Copy link
Copy Markdown

Summary

Fix ssh2_shell() argument validation and make shell and exec environment handling safe and consistent.

  • Accept complete width/height pairs and reject width-only calls with a warning and false.
  • Traverse environment keys and values with one paired Zend hash iterator.
  • Handle empty shell environment arrays without touching invalid hash state.
  • Keep environment values out of setenv failure warnings.

Details

The existing arity check tests five arguments, which rejects a valid width/height call and leaves the incomplete four-argument form unchecked. It also raises E_ERROR, making the documented false return unreachable.

The shell environment loop uses the array's internal pointer, while the exec loop advances an external position for keys but reads values from the internal pointer. Empty shell arrays can reach invalid iterator state, and exec can pair later keys with the wrong value.

Both paths now use ZEND_HASH_FOREACH_STR_KEY_VAL. Numeric keys retain the existing notice behavior. Failed environment requests still identify the variable, but no longer include its value.

Tests

Added PHPT coverage for:

  • valid and incomplete shell dimensions;
  • empty shell environments;
  • paired exec environment iteration;
  • shell and exec warning redaction.

The extension builds cleanly, the package manifest validates, and the branch merges cleanly with the other open SSH2 fixes in #102, #103, and #104.

Validate the actual width-without-height argument count in ssh2_shell and report the invalid call as a recoverable warning. Complete width and height pairs now reach the normal shell path.

Traverse shell and exec environment arrays with paired Zend hash iterators. This makes empty shell environments safe and prevents exec keys from being combined with values from a different iterator position.

Keep environment values out of setenv failure warnings while retaining the variable name for diagnosis. Add PHPT coverage for dimension validation, empty and multi-value environments, and warning redaction, and include the tests in the PECL package manifest.
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.

1 participant