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

Parser content typehint [strict_types=1] #4412

Closed
John-Betong opened this issue Mar 11, 2021 · 2 comments · Fixed by #4548
Closed

Parser content typehint [strict_types=1] #4412

John-Betong opened this issue Mar 11, 2021 · 2 comments · Fixed by #4548

Comments

@John-Betong
Copy link

John-Betong commented Mar 11, 2021

Info
I have been trying for over a week to trace a new site's "Network connection Error" from going down overnight. No errors in logs so added "strict_types=1" to all PHP app and system PHP files, also set .env -> CI_ENVIRONMENT = development and eventually managed to get some errors reported.

Describe the bug
incorrect type being passed.

CodeIgniter 4 version
CI_VERSION 4.1.1

Affected module(s)
File: ./system/View/Parser.php
[code]
protected function replaceSingle($pattern, $content, $template, bool $escape = false): string
{
// Any dollar signs in the pattern will be misinterpreted, so slash them
$pattern = addcslashes($pattern, '$');

	// Flesh out the main pattern from the delimiters and escape the hash
	// See https://regex101.com/r/IKdUlk/1
	if (preg_match('/^(#)(.+)(#(m?s)?)$/s', $pattern, $parts))
	{
		$pattern = $parts[1] . addcslashes($parts[2], '#') . $parts[3];
	}

	// Replace the content in the template
	$template = preg_replace_callback($pattern, function ($matches) use ($content, $escape) {
		// Check for {! !} syntax to not escape this one.
		if (strpos($matches[0], '{!') === 0 && substr($matches[0], -2) === '!}')
		{
			$escape = false;
		}
		# KLUDGE - 2021-03-08 - force string instead of integer parameter
		return $this->prepareReplacement($matches, (string) $content, $escape);
	}, $template);

	return $template;
}

[/code]

Expected behavior, and steps to reproduce if appropriate
I did not expect the web page to crash :(

Context
OS: [Ubuntu 20.04.1]
Web server [Apache 2]
PHP version [8.0.3]

@John-Betong John-Betong added the bug Verified issues on the current code behavior or pull requests that will fix them label Mar 11, 2021
@MGatner MGatner changed the title Bug: Bug: Parser content typehint Mar 11, 2021
@MGatner
Copy link
Member

MGatner commented Mar 11, 2021

I'm going to respond here in general for all these issues. You are creating a lot of work for the team in place of what could be a very quick and simple Pull Request. I you are unfamiliar or uncomfortable with git you could do these PRs directly from the GitHub interface.

I know you are a strong advocate of strict typing. The framework has moved greatly in the direction since implementing static analysis, but version 4 will never be guaranteed to be "strict safe" so if you are modifying system files to add that functionality you are creating your own bugs, not uncovering existing ones. I would be more charitable about this if you were creating PRs or well-formatted Issues (when appropriate) but I cannot have you generating work for an already tight team.

Take responsibility for these changes if you want to see them happen.

@John-Betong
Copy link
Author

Regret raising these issues, I am not familiar with Git and thought they were issues about not Pull Requests.

As mentioned I have a new site and having problems for over a week and thought they were issues.q

Please delete all issues and I hope to revise with Pull Requests soon.

@paulbalandan paulbalandan removed the bug Verified issues on the current code behavior or pull requests that will fix them label Mar 12, 2021
@paulbalandan paulbalandan changed the title Bug: Parser content typehint Parser content typehint [strict_types=1] Mar 12, 2021
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 a pull request may close this issue.

3 participants