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

List of Core Hacks still necessary in 1.8.0-beta #573

Closed
q2apro opened this issue Dec 17, 2017 · 4 comments
Closed

List of Core Hacks still necessary in 1.8.0-beta #573

q2apro opened this issue Dec 17, 2017 · 4 comments

Comments

@q2apro
Copy link

q2apro commented Dec 17, 2017

Most of my issues I solved already with plugins, however, there is still work (core hacks within qa-include) after each upgrade. This list shows why and what - in the hope that this will be discussed and fixed in the core files:

  1. /app/mailing.php
// q2apro hack for html emails 
// 'body' => trim(qa_opt('mailing_body')) . "\n\n\n" . qa_lang('users/unsubscribe') . ' ' . $unsubscribeurl,
// 'html' => false,
'body' => nl2br( trim(qa_opt('mailing_body'))."\n\n\n" ).' <span style="font-size:12px;color:#888">'.qa_lang('users/unsubscribe').' <a style="color:#888" href="'.$unsubscribeurl.'">Unsubscribe</a></span>',
'html' => true,
  1. /plugins/qa-widget-activity-count.php
$this->output_count($themeobject, qa_opt('cache_acount'), 'main/1_answer', 'main/x_answers');

// q2apro hack: added x percent of questions answered
$solvedperc = 100-( qa_opt('cache_unaqcount')/qa_opt('cache_qcount')*100 );
$themeobject->output('
	<p class="qa-activity-count-item">
		<span class="qa-activity-count-data">'.number_format($solvedperc,2,',','.').' %</span> '.qa_lang('main/solved').'
	</p>
');
// END hack
  1. /plugins/qa-search-basic.php
$titlewordids = qa_array_filter_by_keys($wordtoid, $titlewords);
qa_db_titlewords_add_post_wordids($postid, $titlewordids);

// Add to content words index (including word counts)

// q2apro hack: switch off q2a search to save db space, search taken over by google cse
/*
$contentwordidcounts = array();
foreach ($contentcount as $word => $count) {
	if (isset($wordtoid[$word]))
		$contentwordidcounts[$wordtoid[$word]] = $count;
}

qa_db_contentwords_add_post_wordidcounts($postid, $type, $questionid, $contentwordidcounts);
*/
  1. /pages/register.php
// q2apro added login link for existing users 
$qa_content['custom'] = '
	<p class="or-register">
		Du bist bereits Mitglied? Dann einfach <a href="'.qa_path('login').'">hier einloggen</a>.
	</p>
';

// prepend custom message
$custom = qa_opt('show_custom_register') ? trim(qa_opt('custom_register')) : '';
  1. /pages/activity.php
// q2apro hack: do not show notices like edit or reshown etc.
$questions4 = array();

if ($countslugs) {
  1. /pages/account/account.php
// q2apro hack: disallow change of username
// $changehandle = qa_opt('allow_change_usernames') || (!$userpoints['qposts'] && !$userpoints['aposts'] && !$userpoints['cposts']);
$changehandle = qa_opt('allow_change_usernames');
  1. Hack 1: /db/selects.php
$selectspec = qa_db_posts_basic_selectspec($voteuserid, $full);

// q2apro hack: added sort unanswered questions by upvotes, see unanswered?by=upvotes
// $selectspec['source'] .= " JOIN (SELECT postid FROM ^posts WHERE " . qa_db_categoryslugs_sql_args($categoryslugs, $selectspec['arguments']) . "type=$ AND " . $bysql . " AND closedbyid IS NULL ORDER BY ^posts.created DESC LIMIT #,#) y ON ^posts.postid=y.postid";
if($by=='amaxvote')
{
	$selectspec['source'].=" JOIN (SELECT postid FROM ^posts WHERE ".qa_db_categoryslugs_sql_args($categoryslugs, $selectspec['arguments'])."type=$ AND ".$bysql." AND closedbyid IS NULL ORDER BY ^posts.upvotes DESC LIMIT #,#) y ON ^posts.postid=y.postid";
}
else
{
	$selectspec['source'].=" JOIN (SELECT postid FROM ^posts WHERE ".qa_db_categoryslugs_sql_args($categoryslugs, $selectspec['arguments'])."type=$ AND ".$bysql." AND closedbyid IS NULL ORDER BY ^posts.created DESC LIMIT #,#) y ON ^posts.postid=y.postid";
} 

array_push($selectspec['arguments'], $type, $start, $count);

$selectspec['sortdesc'] = 'created';

// q2apro hack for sort unanswered by upvotes
if($by == 'amaxvote')
{
	$selectspec['sortdesc'] = 'upvotes';
}

return $selectspec;
  1. Hack 2: /db/selects.php
// q2apro hack: do not show unanswered questions in related questions, added: "WHERE (^posts.closedbyid IS NULL)"
$selectspec['source'] .= " JOIN (SELECT postid, SUM(score)+LOG(postid)/1000000 AS score FROM ((SELECT ^titlewords.postid, LOG(#/titlecount) AS score FROM ^titlewords JOIN ^words ON ^titlewords.wordid=^words.wordid JOIN ^titlewords AS source ON ^titlewords.wordid=source.wordid WHERE source.postid=# AND titlecount<#) UNION ALL (SELECT ^posttags.postid, 2*LOG(#/tagcount) AS score FROM ^posttags JOIN ^words ON ^posttags.wordid=^words.wordid JOIN ^posttags AS source ON ^posttags.wordid=source.wordid WHERE source.postid=# AND tagcount<#) UNION ALL (SELECT ^posts.postid, LOG(#/^categories.qcount) FROM ^posts JOIN ^categories ON ^posts.categoryid=^categories.categoryid AND ^posts.type='Q' WHERE ^categories.categoryid=(SELECT categoryid FROM ^posts WHERE postid=#) AND ^categories.qcount<#)) x WHERE postid!=# GROUP BY postid ORDER BY score DESC LIMIT #) y ON ^posts.postid=y.postid 
WHERE (^posts.closedbyid IS NULL) ";
  1. /db/maxima.php
'QA_DB_RETRIEVE_COMPLETE_TAGS' => 1500, // q2apro: before 1000
  1. /ajax/asktitle.php
// SESSION FIX FOR https://github.com/q2a/question2answer/issues/440
$userid = qa_get_logged_in_userid();
  1. /ajax/answer.php
// q2apro hack: allow answers on closed questions, otherwise their content is lost
// if (@$question['basetype'] == 'Q' && !isset($question['closedbyid']) && !qa_user_post_permit_error('permit_post_a', $question, QA_LIMIT_ANSWERS)) {
if ((@$question['basetype']=='Q') && !qa_user_post_permit_error('permit_post_a', $question, QA_LIMIT_ANSWERS)) {
  1. Hack 1 /qa-base.php
// q2apro hack: better htmLawed
// $safe = htmLawed($html, array(
	// 'safe' => 1,
	// 'elements' => '*+embed+object-form',
	// 'schemes' => 'href: aim, feed, file, ftp, gopher, http, https, irc, mailto, news, nntp, sftp, ssh, telnet; *:file, http, https; style: !; classid:clsid',
	// 'keep_bad' => 0,
	// 'anti_link_spam' => array('/.*/', ''),
	// 'hook_tag' => 'qa_sanitize_html_hook_tag',
// ));

$safe = htmLawed($html, array(
	'safe' => 1,
	'elements' => 'img, a, p, br, span, b, strong, i, em, u, sub, sup, strike, table, caption, tbody, tr, td, ul, ol, li',
	'schemes' => 'href: ftp, http, https; *:file, http, https; style: !',
	'keep_bad' => 0,
	// 'anti_link_spam' => array('/.*/', ''), // do not set rel=nofollow
	'hook_tag' => 'qa_sanitize_html_hook_tag',
	'deny_attribute' => 'class, id', // do not allow class and id
));
  1. Hack 2 /qa-base.php
if ($element == 'a' && isset($attributes['href']) && $qa_sanitize_html_newwindow)
	$attributes['target'] = '_blank';

// q2apro hack: only allow certain css styles
if (isset($attributes['style'])) 
{
	$css = explode(';', $attributes['style']);
	$style = array();
	foreach ($css as $v) 
	{
		if (($p = strpos($v, ':')) > 1 && $p < strlen($v)) 
		{
			$prop_name = trim(substr($v, 0, $p));
			$prop_val = trim(substr($v, $p+1));
			if ($prop_name == 'color' || $prop_name == 'font-weight' || $prop_name == 'text-decoration' || $prop_name == 'width' || $prop_name == 'max-width' || $prop_name == 'float' || $prop_name == 'margin') { // || $prop_name == 'background-color' 
				$style[] = "$prop_name: $prop_val";
			};
		};
	};
	if (!empty($style))
	{
		$attributes['style'] = implode('; ', $style);
	} 
	else 
	{
		unset($attributes['style']);
	};
};
// end hack
  1. qa-blob.php
case 'swf':
	header('Content-Type: application/x-shockwave-flash');
	break;

// q2apro: added SVG format 
case 'svg':
	header('Content-Type: image/svg+xml');
	break;
@svivian
Copy link
Collaborator

svivian commented Dec 18, 2017

Thanks for the list, I’ll respond to each of them. Note I’m on mobile so I can’t look up what every change does, so I might misunderstand some things.

  1. Yes, would be great to have HTML emails. Main problem is the current system of using language files makes it difficult to work with HTML. If we switch to a templating language (eg Twig) it would be much easier.

  2. I understand you see this kind of thing as a core hack, but if you want something different to the default plugins then that is a different plugin. Otherwise we could end up with dozens of options controlling all sorts of stats. No harm in making your own plugin with different stats available.

  3. Not sure why this needs to be a core hack, you can replace the search box in your theme.

  4. I don’t understand this one. We already have login links on each page. You can also add your link in a widget.

  5. If we remove those things then it wouldn’t be the activity page. If you want a different page then you can make a page plugin showing what you want.

  6. There’s already an option to prevent changing of usernames.

  7. As above, can be done with a page plugin. I’m not sure off-hand whether that particular sorting would be very efficient.

  8. Personally I don’t see the advantage, surely you want unanswered questions there so people will go and answer them? As above, you can make a new plugin.

  9. Pretty sure you can override this in your config.

  10. Answered this elsewhere.

  11. We discussed previously. The change is insecure, so we need to fix it a different way.

  12. Will look into this but it seems like the tags are too restrictive (for example no h1-6, thead, dl tags).

  13. Not sure exactly what this is doing but I think styles can be controlled by htmlawed.

  14. Could be useful.

@q2apro
Copy link
Author

q2apro commented Dec 18, 2017

  1. Yes, would be great to have HTML emails.

Why not to give at least an option for html emails for the admin and then switch to the proposed code?

  1. different to the default plugins then that is a different plugin

Alright, will create my own plugin for this. Thanks.

  1. Not sure why this needs to be a core hack, you can replace the search box in your theme.

Because the database gets filled up heavily when qa_db_contentwords_add_post_wordidcounts() is activated.

  1. I don’t understand this one.

Oh, I left German. It says "Not yet member, register here." - In case somebody clicks on login. Good usability says: Provide a login link from register page, and provide a register link from login page.

  1. If we remove those things then it wouldn’t be the activity page. If you want a different page then you can make a page plugin showing what you want.

Basically you are right. But having 100+ edits per day does not interest my community. The activity page is the main page for most, what is interesting is: Questions, answers, comments. I still vote for getting rid of the edit notices, or at least an admin option. Implementation is very easy, as shown: $questions4 = array();

  1. There’s already an option to prevent changing of usernames.

But it says not only "do not allow the username change" for all. Current code says: "do not allow, but allow for users that have no posts." which is not the same. I hope we will soon have the implementation by userids, e. g. /user/123/mycustomname, then I would not care about this option at all.

  1. not sure off-hand whether that particular sorting would be very efficient.

By sorting unanswered questions by upvotes you find out which questions are the most interesting ones! Should definitely go into core. Proven as very helpful, even on stackoverflow.

  1. Personally I don’t see the advantage, surely you want unanswered questions there so people will go and answer them?

No, from my community I see that either experienced users look there to post similar questions to the OP, or the OP finds a similar answer on the way. I cannot remember a single case where someone looked to related questions to answer them.

  1. Pretty sure you can override this in your config.

Ah damn, I forgot about this. Thanks a lot!

  1. Answered this elsewhere.

Alright. Done and removed the hack.

  1. We discussed previously. The change is insecure, so we need to fix it a different way.

Why insecure? I hope you remember that this happens (several times in my forum): Incoming question, user Z writes his answer, meanwhile user A closes the question, user Z submits his answer. Page loads. Answer completely lost! The hack is the only way how to rescue the content.

  1. Will look into this but it seems like the tags are too restrictive (for example no h1-6, thead, dl tags).

Sure, could be added. Or as admin option? For me, this filter is until now very helpful.

  1. Not sure exactly what this is doing but I think styles can be controlled by htmlawed.

This allows certain inline styles. More info here.

  1. Could be useful.

Maybe ignore this one, as SVG have security issues. It was just for completeness :) I even thought of removing it myself again.

@svivian
Copy link
Collaborator

svivian commented Dec 18, 2017

Why not to give at least an option for html emails for the admin

Mainly because I haven't had time to look into it. Features take time to implement and involve more than just changing 'false' to 'true' (especially in terms of security). I've been spending my time in other areas.

Because the database gets filled up heavily

I was under the impression that a custom search plugin would disable the built-in search but it doesn't look like that is the case. Perhaps the built-in search could be moved out into qa-plugins/ to let it be disabled.

It says "Not yet member, register here." - In case somebody clicks on login.

Makes sense, seems like most sites also do this.

Current code says: "do not allow, but allow for users that have no posts." which is not the same.

Is this really a problem? If they have no posts yet then changing their username doesn't really mean anything.

Why insecure?

Because closed questions cannot be answered, and this allows anyone to answer them. It's the equivalent of doing form validation only in JavaScript and not server side.

@q2apro
Copy link
Author

q2apro commented Jan 15, 2018

Because closed questions cannot be answered.

Then I suggest to check the age of the closed notice - and if it is older than e. g. 12 hours, not allow the posting anymore. But I don't see any big problem here letting answers through. Frontend there are no buttons if there is a closed question. And it must be a developer to be able to do a submit for this case.

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

No branches or pull requests

2 participants