| Author |
 Topic  |
|
|
SiSL
Average Member
Turkey
671 Posts |
|
|
AnonJr
Moderator
United States
5768 Posts |
 Posted - 08 April 2009 : 15:59:17
|
| Not to be a smartass, but what problem does this cause? I ask only because I saw this behavior here and didn't see any problems... |
 |
|
|
SiSL
Average Member
Turkey
671 Posts |
|
|
HuwR
Forum Admin
United Kingdom
20584 Posts |
Posted - 08 April 2009 : 18:05:12
|
actually all you need to do is
if intPageNumber > int(intPageNumber) then
intPageNumber = int(intPageNumber) + 1
end if
there is no point in converting it to a Long first unless of course you have topics with > 36000 pages
not sure why cLng does not work thogh, since cLng should NOT retain the decimal |
|
|
|
SiSL
Average Member
Turkey
671 Posts |
Posted - 08 April 2009 : 18:36:45
|
Well, I just kept "cLng" in the fix because thought it may be there for other reasons that I may not be aware of. Removed it now.
First idea is I think cLNG deals with Locale Settings of the Servers. (Like taking 3,6 as currency subtype rather than decimal as 3.6, I may be quite wrong on this theory) However, cInt also does not work, so I'm guessing it has very same issue about the variable to convert. But int does work.
And second as more plausible, unlike Int or Fix, when the fractional part is exactly 0.5, the CLng function always rounds it to the nearest even number. For example, 0.5 rounds to 0, and 1.5 rounds to 2. Even THAT may be problem to see that for instance: if intPageNumber equals to 3.5 never gets greater than cLng(3.5) which is 4 for example intPageNumber > cLng(intPageNumber) would never trigger... It MAY redirect to wrong page even. So you are right, cLng should be removed there even.
So if second idea is at place here, I think there might be other places to check (like places cLng is used to prevent SQL Injections etc.) that might lead to wrong numbers while "rounding" but I don't see anywhere else in a quick look.
PS: I don't remember this happening on 3.4.0.5 (last version I modified) I think. |
CHIP Online Forum
My Mods
Select All Code | Fix a vulnerability for your private messages | Avatar Categories W/ Avatar Gallery Mod | Complaint Manager
Admin Level Revisited | Merge Forums | No More Nested Quotes Mod
|
Edited by - SiSL on 08 April 2009 19:23:54 |
|
|
|
AnonJr
Moderator
United States
5768 Posts |
Posted - 08 April 2009 : 20:19:08
|
I suspect that cLng() was used to keep from running into issues when you get a page number that is larger than what cInt() can handle. You would think that shouldn't be an issue, but since you can shorten the number of replies per page - and there are people who make them as short as the audience will tolerate - it is a distinct possibility.
Having said all that, cLng() should not be producing a decimal regardless of the server settings. Just out of curiosity, what version of IIS are you running this on? And what version of VBScript are you using? |
|
|
|
AnonJr
Moderator
United States
5768 Posts |
Posted - 08 April 2009 : 20:34:37
|
quote:
Originally posted by SiSL
PS: I don't remember this happening on 3.4.0.5 (last version I modified) I think.
I don't think we had the redirection code running 3.4.05. Though I do remember seeing this issue in 3.4.06 - but since I didn't see problems, I didn't think anything of it.
quote:
1. It creates false URLs for search engines (identical pages with different URL's)
Can't speak to this one until I dig around and see if you can get the same page with different numbers. Probably so, but unless you're a SEO junkie I don't know that it will cause tremendously horrid issues. But that may be a different discussion for a different thread.
Through all this though, it seems that I've always seen the fractional page number in all the "jump to" links and I've not seen it produce the whole number for the link. While an issue that needs to be addressed, I don't see where you're getting two different URLs for the same page - just the potential for such.
quote:
2. It's a line that does not do its purpose that it is written to do (that intPageNumber will be always equal to clng(intpagenumber) and never greater.
I'll have to dig into how intPageNumber is generated, but it seems that since this code exists, it was figured that it could be a fractional number - thus the check. Either way, I need to look at the code in context to see what else is being done with it. It may be that this is not where the real problem lies.
quote:
3. It messes up with Rewrite URL of IIS7 and Mod Rewrite components.
Honestly, this is really only a problem if you're trying to make "friendly" URLs - something that IMHO is overrated as only a few technically inclined power users really even look at the URL - much less dissect it. But that aspect is - again - another discussion for a different thread. If you really want to go tilting at that windmill...
Now after that "I've probably been hanging around HuwR too long" response, you are right in that we shouldn't be getting the fractional number in the URL. I'm just not convinced that the code you posted is the culprit. |
|
|
|
SiSL
Average Member
Turkey
671 Posts |
Posted - 08 April 2009 : 20:52:17
|
quote:
Originally posted by AnonJr
...
Well, there was still jump to lastpost redirection with TOPIC_ID=xxx&REPLY_ID=yyy&whichpage=-1 code in 3.4.0.5 too, so that had to be there...
About Search Engines, yes, as a SEO junkie I can easily say: "Search Engines takes everything as new page that is within Query String" and follow Redirects (otherwise, they would not crawl through topic.asp?TOPIC_ID=xxx :) Since whichpage function is also in QuerySTring itself and jump to last post links are also within reach of crawling, 1.) it crawls through them, 2.) It marks them as duplicate content which has negative effects on SEO. Same goes for "Friendly URLs", well, This is not about users, it is more about SEO again. Second most important place of a page is URL, after the Page Title and everything in URL is another keyword.
However, URL Rewrite is one of great ways to prevent many injection etc. attempts as well, considering RegEx'd URL paths (for instance only allowing numbers in part of an URL etc)
And as final, I'm using IIS6.0 with Turkish Locale. But since you have noticed that HERE too, I'm guessing it has more to do than Locale. (This does not happen on my live forum btw, I didnot upgraded it yet to 3.4.0.7, noticed that on my new forum design using a duplicated DB of live one on a soapbox server)
I just noticed that part and wanted to inform, at least there is something weird going on there.
Since you want to solution with cLng not Int, here goes:
if intPageNumber > cLng(intPageNumber) then
intPageNumber = cLng(intPageNumber) + 1
else
intPageNumber = cLng(intPageNumber)
end if
1. We check if intPageNumber > cLng(intPageNumber) , so intPageNumber can be decimal...
2. But we don't also convert it back to cLng if it is not greater than cLng of it...
Naturally for example if intPageNumber is 14.7, cLng(14.7) = 15, however, same page example, 14.3 is cLng(14.3) = 14, both should be in same page, right? But in 14.3 example, intPageNumber > cLng(intPageNumber) does verify, so we still get page number with adding + 1 in the function, but in 14.7 example it does not verify, it is still lesser than it is cLng(14.7), therefor, we still need to convert 14.7 intPageNumber to some integer or long.
So main PROBLEM here WAS cLng is a rounding function rather than taking digits before decimals like int does. It rounds the number up or down (which is why I'm anxious now, since I used to use it for verifying input too). I think confusion was thinking cLng works like int from previous code. I just learned something new on this.
I'm going to go ahead and change topic with this for now and ask for your help if it is the correct solution to this.
|
CHIP Online Forum
My Mods
Select All Code | Fix a vulnerability for your private messages | Avatar Categories W/ Avatar Gallery Mod | Complaint Manager
Admin Level Revisited | Merge Forums | No More Nested Quotes Mod
|
Edited by - SiSL on 08 April 2009 23:06:53 |
|
|
|
ruirib
Snitz Forums Admin
Portugal
26364 Posts |
|
|
HuwR
Forum Admin
United Kingdom
20584 Posts |
Posted - 09 April 2009 : 01:26:15
|
| I posted the correct solution in my reply |
|
|
|
Shaggy
Support Moderator
Ireland
6780 Posts |
Posted - 09 April 2009 : 03:57:53
|
Another solution is on page 2 of the original topic which, for some reason, never made it into the base code.
|
 Search is your friend
“I was having a mildly paranoid day, mostly due to the
fact that the mad priest lady from over the river had
taken to nailing weasels to my front door again.” |
|
|
|
SiSL
Average Member
Turkey
671 Posts |
|
| |
Topic |
|
