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

url greater than 255 chars are not saved (was: urls with redirection(s) silently fail to be downloaded) #681

Closed
pmaziere opened this issue May 12, 2014 · 11 comments
Assignees
Labels
Milestone

Comments

@pmaziere
Copy link

When sending wallabag a url that ends up to be a redirection (HTTP 3xx) to another url, it seems that wallabag does not follow the redirection, and page download fails silently wihout even a failure notice

@tcitworld
Copy link
Member

Hi,
Could you give an example of page ?
I used for instance the goo.gl service which gives HTTP 301 redirection and it works for me.

@pmaziere
Copy link
Author

@tcitworld
Copy link
Member

Well, I've been trying for 15 minutes with no success, but it works now. I guess it was a problem on MetroNews' side.

Can you confirm it ?

@pmaziere
Copy link
Author

nope it does not work
this link leads to 2 redirections before reaching the final page: I can always get to the final page when entering the link in my browser address bar, but it always fail to be downloaded with wallabag.

I did not check in details what wallabag was doing, but I guess I will have to get a closer look since you are not able to reproduce this issue

@tcitworld
Copy link
Member

Well, it didn't work at the beginning, as with the original Google URL and with the final MetroNews URL : http://www.metronews.fr/info/et-si-vous-controliez-vos-reves-grace-a-l-electricite/mnel!0B6VnlzEtMG6w/ but now they both work. I think they had issues on their side because once I got a 500 server error.
Try again in a few hours in case.

@pmaziere
Copy link
Author

2014-05-12 17:55:33: (mod_fastcgi.c.2676) FastCGI-stderr: DEBUG POCHE : storage type postgres
DEBUG POCHE : display home view

2014-05-12 17:55:50: (mod_fastcgi.c.2676) FastCGI-stderr: DEBUG POCHE : storage type postgres
DEBUG POCHE : execute query error : SQLSTATE[22001]: String data, right truncated: 7 ERROR: value too long for type character varying(255)
DEBUG POCHE : error during insertion : the link wasn't added http://news.google.com/news/url?sa=t&fd=R&ct2=fr&usg=AFQjCNHJgjNo3aondjF0E3S_vUEQ3o4Y5A&clid=c3a7d30bb8a4878e06b80cf16b898331&cid=52778860338942&ei=vrNwU9iCO4e1iAatCg&url=http://www.metronews.fr/info/et-si-vous-controliez-vos-reves-grace-a-l-electricite/mnel!0B6VnlzEtMG6w/
DEBUG POCHE : redirect to ?view=home&closewin=true

it seems to be an issue with postgresql "entries" table definition: url is limited to 255 characters while this url is 274. This is also the case for mysql, while the TEXT data type in sqlite does not seem to be limited. I guess there should not be any limit to url and title length

so for postgresql and mysql, VARCHAR(255) should be replaced by TEXT

@tcitworld
Copy link
Member

Okay. I was using it with SQLite which has no character limitation, that's why.
I wonder if we better save the final URL into DB rather than the given one in case of redirections.
But you're right however, we've got to change this into a TEXT field.

@tcitworld tcitworld added the Bug label May 12, 2014
@pmaziere
Copy link
Author

It depends on the redirection: if it's a 301 or 308 error code, then the final URL should be saved.
Otherwise the first URL should be considered as a permanent link and the redirection as a link that may change in the future for whatever reasons

@pmaziere pmaziere changed the title urls with redirection(s) silently fail to be downloaded url greater than 255 chars are not saved (was: urls with redirection(s) silently fail to be downloaded) May 12, 2014
@tcitworld
Copy link
Member

That's totally right.

@tcitworld tcitworld added this to the 1.8.0 milestone Jun 5, 2014
@tcitworld tcitworld modified the milestones: 2.0, 1.8.0 Jun 30, 2014
@tcitworld
Copy link
Member

Since it implies database changes, this goes to v2 (a upgrade script would have been required either).

@nicosomb nicosomb modified the milestones: 1.8.0, 2.0 Jul 13, 2014
@nicosomb nicosomb mentioned this issue Jul 13, 2014
18 tasks
@tcitworld tcitworld modified the milestones: 1.8.1, 1.8.0 Oct 1, 2014
@nicosomb nicosomb modified the milestones: 1.8.1, 1.8.2 Oct 29, 2014
@tcitworld tcitworld modified the milestones: 2.0, 1.8.2 Dec 22, 2014
@j0k3r
Copy link
Member

j0k3r commented Mar 28, 2015

url column are now defined as text in the v2 branch.

@ORM\Column(name="url", type="text", nullable=true)

@j0k3r j0k3r closed this as completed Mar 28, 2015
@nicosomb nicosomb removed this from the 2.0 milestone Aug 12, 2015
@nicosomb nicosomb modified the milestones: 2.0.0-alpha, 2.0 Aug 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants