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

loop 문에서 함수를 2번씩 호출하는 버그 #1893

Closed
Erictoby opened this issue Mar 5, 2022 · 4 comments
Closed

loop 문에서 함수를 2번씩 호출하는 버그 #1893

Erictoby opened this issue Mar 5, 2022 · 4 comments
Labels
bug 버그로 확인된 사항 patch available 패치 있음 표시 (bug와 함께 사용)

Comments

@Erictoby
Copy link
Contributor

Erictoby commented Mar 5, 2022

loop 문이 함수를 2번씩 호출하는 버그가 있네요.

$nodes[$idx - 1] .= sprintf('<?php if(%1$s)foreach(%1$s as %2$s){ ?>', $expr_m[1], $expr_m[2]);

605fcc8 커밋에서 에러가 생겼습니다. 그 뒤로, 3d24c14 c728250 에서 조금씩 바뀌어 현재 모습이 된것 같은데, <block loop="..."> 에는 변수나 오브젝트만 들어가는 것이 아니라 함수도 들어가는데, 그냥 실행만 되는 함수도 있지만 호출될때마다 다른 곳에 값을 계속 추가하는 놈도 있습니다. 즉, 2번 실행되어 시간낭비하는 것 뿐만 아니라 결과가 달라지네요.

예를들면,

<block loop="Context::getCssFile(true) => $key, $css_file">

이런 부분이 2번씩 함수를 중복 호출하고, server push를 선택하면 header에 2번씩 집어넣고, nginx는 헤더가 너무 커져서 그런지 처리 못하고 502 에러를 가끔 내고...

예전 것으로 rollback 검토해주시면 감사하겠습니다. (추가: count()는 array가 아닐수 있으니까 빼던지 해야겠네요)

$nodes[$idx - 1] .= sprintf(
'<?php $t%3$s=%1$s;if($t%3$s&&@count($t%3$s))foreach($t%3$s as %2$s){ ?>'
,$expr_m[1], $expr_m[2], md5( $buff . strval($idx-1) )
);

@kijin
Copy link
Member

kijin commented Mar 5, 2022

변수가 존재하고 배열이 맞는지 확인하지 않으면 최근 PHP에서 치명적인 오류가 떠버리기 때문에 저렇게 처리했던 것 같습니다. 이 기능이 없으면 문제가 되는 기존 자료가 많고, 어차피 <block loop="..."> 문법 자체를 전혀 권장하지 않고 있으므로 1) 신규 개발하시는 자료에서는 해당 문법을 사용하지 않으면 그만이고, 2) 기존 자료나 코어 템플릿 중 해당 문법을 사용하는 것이 있으면 제거하고, 3) 차기 버전의 템플릿 문법에서는 <block loop="..."> 를 지원하지 않는 방향으로 진행하겠습니다.

2.1에서 브라우저 지원 범위 변경으로 CSS, JS의 targetie 속성 지원을 중단할 예정이므로, 이 때 common_layout.html의 CSS, JS 로딩 코드도 간소화하겠습니다.

@kijin kijin added this to the 2.1+ milestone Mar 5, 2022
@kijin kijin added cleanup 정리 compatibility 호환성 고려 필요 labels Mar 5, 2022
@Erictoby
Copy link
Contributor Author

Erictoby commented Mar 5, 2022

loop을 쓰면 에디터에서 읽기 편해서 많이 쓰게 된것 같습니다 ㅠㅠ. loop에서 함수를 호출하는 부분을 찾아보니까 comment_layout.html 5곳하고 <li loop="$oDocument->getComments()=>$key,$comment" 라는 부분이 보이네요. 다른 함수들도 몇 있는데 자체적으로 buffering 해서 괜찮을 것 같구요.

php 8의 에러 때문이라면,

sprintf('<?php if(%1$s)foreach(%1$s as %2$s){ ?>', $expr_m[1], $expr_m[2]);

sprintf('<?php $_rx_tmp=%1$s;if($_rx_tmp)foreach($_rx_tmp as %2$s){ ?>', $expr_m[1], $expr_m[2]);

처럼 하면 경고는 남고 에러는 사라지고, $_rx_tmp는 foreach까지만 쓰고 버려지는 변수이므로 cascade되더라도 괜찮으니 예전처럼 md5로 hashing할 필요까지는 없겠죠. 그래도 혹시 몰라 test를 해봤는데 문제는 없네요 (이 글을 나중에 참조할 다른 분들을 위해서 남겨보면),

$testarray = array("1", "2", "3");
$tmp = $testarray;
foreach($tmp as $test1)
{
	$tmp = $testarray;
	foreach($tmp as $test2)
	{
		echo "test1 = $test1, test2 = $test2 <br />";
	}
}

$_rx_tmp=%1$s; 를 $_rx_tmp=(%1$s) ?? ''; 로 하면 경고도 사라지겠지만 전에 말씀하셨듯이 경고가 있는편이 디버깅에 도움이 될수도 있겠지요.

@kijin
Copy link
Member

kijin commented Mar 5, 2022

@Erictoby 임시변수 미리 세팅해 놓는 것 괜찮네요. 감사합니다.^^

@kijin kijin removed this from the 2.1+ milestone Mar 13, 2022
@kijin
Copy link
Member

kijin commented Mar 13, 2022

알려주신 방법대로 임시변수를 사용하여 해결했습니다.

@kijin kijin added bug 버그로 확인된 사항 patch available 패치 있음 표시 (bug와 함께 사용) and removed cleanup 정리 compatibility 호환성 고려 필요 labels Mar 13, 2022
@kijin kijin closed this as completed in f373e38 Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 버그로 확인된 사항 patch available 패치 있음 표시 (bug와 함께 사용)
Projects
None yet
Development

No branches or pull requests

2 participants