taskGNU Herds - Free Software Association - Tasks: task #6181, New feature: entity photo

 
 

You are not allowed to post comments on this tracker with your current authentication level.

task #6181: New feature: entity photo

Submitter:  Davi Leal <davi>
Submitted:  Sun 03 Dec 2006 11:47:06 AM UTC
   
 
Should Start On:  Sun 03 Dec 2006 12:00:00 AM UTC Should be Finished on:  Tue 02 Jan 2007 12:00:00 AM UTC
Category:  New Feature Priority:  3 - Low
Status:  Done Privacy:  Public
Assigned to:  davi Percent Complete:  100%
Open/Closed:  Closed Effort:  0.00

Jump to the original submission

Mon 25 Dec 2006 12:53:02 PM UTC, comment #11: 


> About the $array[30]: I did understand this, and, in fact,
> I left all $array[30] and $array[31] unchanged. ;)


To data which come from other table via foreign keys, for example,
  // LO_Name for E1_LO_Country
I used the index 30, 31, etc, leaving free index to use 19..29 if we add field in the 'pather' table.

We can fix it in the next big realease, 2.0, if it is needed.


> as you already stated, we are a beta project, and can
> always fix things later.


Agreed. We can discuss this point when we begin the 2.0 version.


P.S.: Close the new task 'entry point ..' if it is fixed.
   Status: Done

Davi Leal <davi>
Group administrator
Mon 25 Dec 2006 11:38:01 AM UTC, comment #10: 

Good.

About the $array[30]: I did understand this, and, in fact, I left all $array[30] and $array[31] unchanged. ;)

About checking before committing: I know, sorry. But look for my "Check site entry point" task. I explain everything there.

About the security problems. Well, I'm not sure either. But it's best, imho, to have things correctly coded. But, as you already stated, we are a beta project, and can always fix things later.

David Paleino <hanska>
Mon 25 Dec 2006 11:32:19 AM UTC, comment #9: 

I have checked. It works!. It must be always checked before committing.

About other occurrences, at Entity.php getEntity($Id)  note that we use an $array[30] but no 29 neither 28, etc. I designed it so to be able to add more table field after the  $array[19] =

I am not sure, leaving such array-indexes not used be a security problem.  Please, leave it as it is now. We should not do such source code modifications at the current state of the project 'Beta'. We could add bugs. If we confirm it is a security problem, we will fix it immediately, else we can improve it when we be almtost sure that we will not add more fields to that table.

About the security image, I agree with you about not to add the 'no photo' string.

Davi Leal <davi>
Group administrator
Mon 25 Dec 2006 10:39:38 AM UTC, comment #8: 

I've fixed things with $array[11]. It still needs testing though.

These are the files I edited with my last commit:

./Layer-2__Business_logic/content/forms/Company_form.php
./Layer-2__Business_logic/content/forms/Person_form.php
./Layer-2__Business_logic/content/forms/View_Job_Offer_form.php
./Layer-2__Business_logic/content/forms/View_Qualifications_form.php
./Layer-2__Business_logic/content/forms/non-profit_Organization_form.php
./Layer-5__DB_operation/Entity.php
./Layer-5__DB_operation/Job_Offer.php

If you find any other occurrences, please tell me.

About the default image: I've just seen the ones in Layer-0__Site_entry_point/images/default. They're nice! I don't think we need adding a "no photo" string over them.

David Paleino <hanska>
Mon 25 Dec 2006 08:50:37 AM UTC, comment #7: 

OK, start editing the code to fix this $array[11] issue. Maybe there is other cases as this. But we can begin fixing this. Cool ;)


About index.php, we proposed be it a link. Right now it is a link to the Home.php page, but it could later link to FS_Job_Offers.php, FAQ.php or any new section.

We could even execute a random script each week to link the index.php to one the of more important section and less viewed web pages.


About the default image, there is already such seudo code at:

  {if $smarty.session.ViewPhoto eq '' }

 Layer-0__Site_entry_point/templates/View_Job_Offer_form.tpl
 Layer-0__Site_entry_point/templates/View_Qualifications_form.tpl


We could update the default image which is being showed now and use one more appropriate, if needed. Or we could add to the current default a string over it as "no photo". We could even translate that string due to we could use ImageMagic http://www.imagemagick.org/script/index.php or something similar to add a translated string to the default photo.


--
About taking over this tasks, it is OK for me taking it. You can help me at any time or ask for taking it over later.

Davi Leal <davi>
Group administrator
Sun 24 Dec 2006 11:26:50 PM UTC, comment #6: 

"_I have checked that $array[11] at Entity.php getEntity($Id) is not used, but I have not modified the index numbering to match left-right-assign due to I would need to update a lot of files."

Well, I think we really should fix this. It's an improvement of code readability, and could expose the app to possible bugs. Imagine that we actually have a 21-long array, with 20 effective values (remember that, even if $array[11] is not defined, it's still present, and its value is just "").
The problem of updating lots of files is relative: I've checked the files to edit too, and the result is:

./Layer-2__Business_logic/content/forms/Company_form.php:223-229
./Layer-2__Business_logic/content/forms/non-profit_Organization_form.php:222-228
./Layer-2__Business_logic/content/forms/View_Qualifications_form.php:163-175
./Layer-2__Business_logic/content/forms/Person_form.php:229-237
./Layer-5__DB_operation/Job_Offer.php:490-499

In particular, the indicated lines refer to the $array[12] -> $array[20] values in Entity.php->getEntity($id). As you can see, it's less than 10 lines per file, per 5 files => less than 50 lines to edit to keep code clean. Please tell me if I'm missing something, obvious or not (if I'm not missing anything, just tell me, so that I can start editing the code to fix things up).

Moreover, I've just tried to run the webapp with my apache2 installation. It didn't work simply because: it doesn't have an index.php! How is it supposed to "start"?

"_ I think that the png streaming function will fail if the check fails, but I am note sure:

If the there is E1_Id image file and the format check does not fail the entity has photo and we stream it to the client browser._"

Well, we can always think of a default image, in case the entity does not upload one. An image showing "No photo uploaded" should be good. Something like (pseudo-code):

if ($check) {
    fread($my_image);
} else {
    fread($default_image);
}

"_P.S: If you want to realize this task take over it. So I will able to carry out this one: http://savannah.nongnu.org/task/?6266 We can work as a team, if I need help I can ask to you, if you have time and want .., and vice-versa. _"

Well, I'd like to: the problem is that I need some more time to understand how everything works. I've never designed a website this way, the "Layer" way I mean. I've always created my functions in an ./inc/ directory, and used those all over the website. Now I need some time to get used to this Layer-thing. ;)
I can start working on it. But I don't feel like taking it over, at least for the moment.

Cheers,
David

David Paleino <hanska>
Sun 24 Dec 2006 08:20:20 PM UTC, comment #5: 

First, and maybe last time, that I will do a so long description of the patch which I have committed. It is just so you can take a look of what I do:



Checking the webapp before committing the database field deletion, I have seen it fails!.

  Layer-5__DB_operation$ grep E1_Photo *.php

I have checked that $array[11] at Entity.php _ getEntity($Id) is not used, but I have not modified the index numbering to match left-right-assign due to I would need to update a lot of files. The patch is more clean without modifying it. That is to say, it is so:

                $array[10] = pg_fetch_all_columns($result, 10);
        REMOVED       [11]
                $array[12] = pg_fetch_all_columns($result, 11);
                $array[13] = pg_fetch_all_columns($result, 12);
                $array[14] = pg_fetch_all_columns($result, 13);
                $array[15] = pg_fetch_all_columns($result, 14);
                $array[16] = pg_fetch_all_columns($result, 15);
                $array[17] = pg_fetch_all_columns($result, 16);
                $array[18] = pg_fetch_all_columns($result, 17);
                $array[19] = pg_fetch_all_columns($result, 18);
                $array[20] = pg_fetch_all_columns($result, 19);
                       20
instead of so:
                $array[10] = pg_fetch_all_columns($result, 10);
                $array[11] = pg_fetch_all_columns($result, 11);
                $array[12] = pg_fetch_all_columns($result, 12);
                $array[13] = pg_fetch_all_columns($result, 13);
                $array[14] = pg_fetch_all_columns($result, 14);
                $array[15] = pg_fetch_all_columns($result, 15);
                $array[16] = pg_fetch_all_columns($result, 16);
                $array[17] = pg_fetch_all_columns($result, 17);
                $array[18] = pg_fetch_all_columns($result, 18);
                $array[19] = pg_fetch_all_columns($result, 19);
                       19

  from_webapp_root$ find . | xargs grep getEntity

  I have checked the resulting files, and I have seen the Smarty condition
  which is being used:

    {if $smarty.session.ViewPhoto eq '' }

  So I replace:
        $_SESSION['ViewPhoto'] = $result[11][0];
  with:
        $_SESSION['ViewPhoto'] = '';

  Later, I check again the webapp.


Extra check:
  from_webapp_root$ find . | xargs grep Photo

  and check again the webapp.


P.S.: I advise you to reload the data base to be sure the new SQL implementation works rightly. The drawback is that you will lost are your loaded data.
  psql gnuherds < GNUHerds__SQL_Implementation.psql > stdout 2> stderr

P.P.S: As a security measure, I will not update the production environment up to I work some days more with this patch applied in my development environment. On production I will execute the below command after the usual  cvs update -d
 
  ALTER TABLE e1_entities DROP e1_photo;

--
M Layer-2__Business_logic/content/forms/View_Job_Offer_form.php
M Layer-2__Business_logic/content/forms/View_Qualifications_form.php
M Layer-5__DB_operation/Entity.php
--

Davi Leal <davi>
Group administrator
Sun 24 Dec 2006 06:56:31 PM UTC, comment #4: 

About the library to do the image streaming, the two below links point to the same documentation page:
  http://es.php.net/gd
  http://es.php.net/image

Davi Leal <davi>
Group administrator
Sun 24 Dec 2006 06:41:30 PM UTC, comment #3: 

I have asked and now I agree with you about using the filesystem option.

  • Scalability:

  With static files you could just hand off to another static server. This will be good when it move to the FSF hosts, if the project success and gets a lot of members.

  • Search speed:

  There are almost no advantages to storing binary data in a database. 

  • Security:

  You can have an insecure database just as you can have an insecure filesystem or vice-versa.

  About public URLs:
  Approaches around this are feeding the files through a gateway, basic http auth, impossible-to-guess filenames with limited access, temp files, etc.

  We could store the file path and stream it to the client browser, as an anonymous file.

  Relative locations stored in the database (but better, don't store the relative path, store some information which can be turned into the file path). We can set the path, host, and protocol to access the file via configuration parameters.

  As file name, we could use the E1_Id, which is an integer and the PRIMARY KEY of the E1_Entities database table. See Layer-0__Site_entry_point/doc/GNUHerds__SQL_Implementation.psql

  Such file could be only accessible by the HTTP user and root of course.

  I think that the png streaming function will fail if the check fails, but I am note sure:

  If the there is E1_Id image file and the format check does not fail the entity has photo and we stream it to the client browser.

  I am deleting right now the below database field, which is not being used now, and will not be needed if we are going to keep the photo as a file:
  E1_Photo bytea, -- Person's photo, or company
                  -- or non-profit Organization photo.


P.S: If you want to realize this task take over it. So I will able to carry out this one: http://savannah.nongnu.org/task/?6266  We can work as a team, if I need help I can ask to you, if you have time and want .., and vice-versa.

P.P.S: I can do mistake. Let me now any disagreement.

Davi Leal <davi>
Group administrator
Sun 24 Dec 2006 12:15:46 AM UTC, comment #2: 

I think the filesystem solution is actually the best one.

Another solution could be saving images as BLOB into the database (sorry, never used PostgreSQL, always MySQL... I hope that there's a similar data type). This solution is, however, expensive for the database server, mainly in terms of database size. This solution should be used only when the images are going to be heavily shared between sites.

Since we aren't sharing these "avatars", we should take into consideration the filesystem option.

We could always use the database to create associations between usernames and images; a better solution would be (imho) using the username as filename: i.e. user "foo", image saved as "foo.png" (or whatever) on the disk. This would let us avoid querying the database. But, on the other hand, could also be a security risk. (if one successfully creates a file named "myuser.foo" on the hdd, which is not an image, what would happen? we should implement also a image-checking algorithm, or something kinda, and use it also with the upload form)

Cheers (and goodnight... yawn.. and merry christmas to anyone in here),
David

David Paleino <hanska>
Fri 08 Dec 2006 10:22:28 AM UTC, comment #1: 

How to show a image got from a DataBase query?. Have we to save it, as a file, in the local filesystem?.

Davi Leal <davi>
Group administrator
Sun 03 Dec 2006 11:47:06 AM UTC, original submission:  

A default photo is being displayed.

The data base register to keep the actual photo is ready:
  E1_Photo  bytea

Davi Leal <davi>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by hanska (Posted a comment)
  • -email is unavailable- added by davi (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 19 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-01-13 davi StatusReady For Test Done
    2007-01-13 davi Open/ClosedOpen Closed
    2006-12-31 davi Priority7 - High 3 - Low
    2006-12-31 davi StatusIn Progress Ready For Test
        Percent Complete90% 100%
    2006-12-30 davi Percent Complete80% 90%
    2006-12-28 davi Should be Finished on2007-01-04 2007-01-02
        Percent Complete70% 80%
    2006-12-28 davi Should be Finished on2007-01-15 2007-01-04
        Percent Complete40% 70%
    2006-12-27 davi Should be Finished on2007-02-01 2007-01-15
        Percent Complete30% 40%
    2006-12-26 hanska CategoryNone New Feature
    2006-12-26 davi Percent Complete20% 30%
    2006-12-25 davi Should be Finished on2007-03-01 2007-02-01
        Percent Complete10% 20%
    2006-12-25 davi Assigned toNone davi
    2006-12-24 davi Priority1 - Later 7 - High
    2006-12-24 davi StatusPostponed In Progress

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code